Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Premultiply font textures for linear blending #1765

Closed

Conversation

felixfaire
Copy link

It is my understanding that egui blends colours in linear space (which is arguably more correct than the SRGB blending of yore). However font textures in egui currently use alpha premultiplied pixels values in srgb space. This leads to darkened text edges when using linear blending, this is particularly noticeable with white text on a white background (see below).

image

This PR changes the font pixel premultiplication function to get (afaict) correctly premultiplied values for linear blending.

image

This problem and solution is well described in this article: https://hacksoflife.blogspot.com/2022/06/srgb-pre-multiplied-alpha-and.html

Notes: The Ubuntu thin font on white backgrounds can appear thinner due to this change (because it removes the grey border around text). I think the result is still legible and looks better but some other users may disagree. A future release may wish to use a slightly thicker default font if this comes up in feedback.

Related:
#1410
#1412
#1411

@felixfaire
Copy link
Author

felixfaire commented Jun 23, 2022

This PR is draft as i'd like to learn more about what is happening in painter.rs here:

                let gamma = if self.is_embedded && self.post_process.is_none() {
                    1.0 / 2.2
                } else {
                    1.0
                };

As i'm not sure my preliminary testing has come up against this edge case yet.

Also, the first version of this PR removes the gamma argument as it is unclear what it's role is other than eyeballing the brightness curve (which would bring back incorrect text borders). I appreciate that this kind of functionality might still be useful so would be interested to hear opinions on it.

@emilk
Copy link
Owner

emilk commented Jul 3, 2022

This effectively reverses #1412

The problem with egui before #1412 was that dark text on white background was too faint. #1412 fixed this, but added the above problem of ugly edges of text.

It of course depends on your use case what problem is the biggest one, but I got many complaints about text being too faint in bright mode, that I believe that problem to be the bigger one.

We should be looking for a solution that fixed both problems at once! Some more clues can be found in #1411

Current master:

master

This PR:

pr1765

@emilk emilk closed this Jul 3, 2022
@felixfaire
Copy link
Author

I appreciate that legible text in light mode is definitely a priority, though i find the solution of hacking the blending to make the thin font work (and look worse for all other fonts in the process) a bit extreme.. I think proper solutions to this might include.

  1. Using an appropriately weighted font for the default font size (Ubuntu regular for example).
  2. Using a darker colour for text in light mode (it's currently set at a relatively light grey).
  3. Making the default font size bigger.

For me (mostly on a macbook) the text is perfectly legible on this PR in light mode (even more-so with black as the font color vs grey). Though i appreciate this isn't for everyone. Perhaps the defaults of egui should change size / color to be more accessible to more users who have an issue with correct blending, then there is always the option to make the font smaller / lighter for users who need the space / density.

I don't consider this issue resolved / PR closed.. So please reconsider looking over this as a serious issue (with solutions outlined above).

@emilk
Copy link
Owner

emilk commented Jul 3, 2022

I agree with you @felixfaire - #1412 is a hack in lieu of a proper fix.

On a high DPI display (e.g. a macbook) all this blending problems becomes much less noticeable simply because the pixels are smaller. So for instance, a 12 point font covers more pixels, so there is more "meat" to each letter.

Before this PR the same font, size and color looked substantially different in egui compared to any other place (especially on low resolution displays), so #1410 is a proper bug that absolutely needed fixing, and I don't want to merge any PR that reopens that rather serious bug.

Your suggestions are great, but they also need to be matched with some fixes in the code along the lines of #1411

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants