search

stevenlovegrove / Pangolin

Pangolin is a lightweight portable rapid development library for managing OpenGL display / interaction and abstracting video input.
MIT License
2.39k stars 855 forks source link

fix font rendering #940

Closed christian-rauch closed 5 months ago

christian-rauch commented 5 months ago

This reverts commit cba1ef20254718ff08cb2b84bc825a2b531d957a.

This fixes the font rendering issue: https://github.com/stevenlovegrove/Pangolin/pull/940#issuecomment-2174216335

Fixes https://github.com/stevenlovegrove/Pangolin/issues/939.

current master PR
pango_sd_master pango_sd_pr
pango_hello_master pango_hello_pr
christian-rauch commented 5 months ago

@stevenlovegrove The "continuous coords" from the commit seem to blur the rendered text (https://github.com/stevenlovegrove/Pangolin/issues/939). What was the reason for changing this? Can this be safely reverted or are we going to introduce issues somewhere else?

mateosss commented 5 months ago

Thanks for the update! I can confirm this fixes #939

stevenlovegrove commented 5 months ago

@christian-rauch looking at the commit, it seems I made an error and should have used ProjectionMatrixOrthographic(-0.5, w-0.5, -0.5, h-0.5, -1.0, 1.0).Load(); instead of the continuous coords.

The reason for the commit was to avoid the extra glMatrixMode calls (since we have to store the projection and model view matrices and then set them in a different method https://github.com/stevenlovegrove/Pangolin/blob/13e4a1804cefe8ddfd22cdddfac183d5d2828859/components/pango_opengl/src/viewport.cpp#L64).

Reverting is safe. We could also replace the ProjectionMatrixOrthographic call with the correct pixel centric version to avoid the two extra GL API calls.

christian-rauch commented 5 months ago

@christian-rauch looking at the commit, it seems I made an error and should have used ProjectionMatrixOrthographic(-0.5, w-0.5, -0.5, h-0.5, -1.0, 1.0).Load(); instead of the continuous coords.

Thanks. This seems to work. I updated the PR.

@mateosss Can you check again with the new commit in the PR?

mateosss commented 5 months ago

Indeed this also fixes the issue, thank you both :)