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

Fix high-DPI scaling in sdl_renderer #427

Merged
merged 3 commits into from
Apr 20, 2022

Conversation

kbolino
Copy link
Contributor

@kbolino kbolino commented Mar 15, 2022

This commit resolves two issues with high-DPI display rendering:

  1. The coordinates were not scaled properly, resulting in tiny output
    and misalignment of actual cursor position with apparent position;
    this is fixed by calling SDL_SetRenderScale with appropriate scaling
    factors determined by comparing the window size to the renderer's
    output size
  2. The fonts were not oversampled, resulting in excessively blurry text;
    this is fixed by setting oversample_h and oversample_v on the
    font_config according to the scaling factors

This commit resolves two issues with high-DPI display rendering:

1. The coordinates were not scaled properly, resulting in tiny output
   and misalignment of actual cursor position with apparent position;
   this is fixed by calling SDL_SetRenderScale with appropriate scaling
   factors determined by comparing the window size to the renderer's
   output size
2. The fonts were not oversampled, resulting in excessively blurry text;
    this is fixed by setting oversample_h and oversample_v on the
    font_config according to the scaling factors
@kbolino
Copy link
Contributor Author

kbolino commented Mar 15, 2022

There's a C89 compliance warning about the inline definition of struct nk_font *default_font which I can fix if desired. This warning was already lurking in some of the commented-out code.

@dumblob
Copy link
Member

dumblob commented Mar 15, 2022

Yeah, some demos are not fully C89 compliant as we're not that strict as with nuklear.h - but if you already found it, feel free to make it compliant 😉

Regarding font blurriness, did you read https://github.com/Immediate-Mode-UI/Nuklear/wiki/Complete-font-guide#font-sharpness ?

@kbolino
Copy link
Contributor Author

kbolino commented Mar 15, 2022

No, I hadn't seen it, thanks for linking it. I was trying to find the font guide earlier, but didn't know there was a wiki. 🤦

Digging into the source a bit, I also learned that the default size of the default font is 13 not 12, and indeed it looks much better on regular displays at size 13 (where it's pixel-perfect).

Trying to make the fonts look good at both low and high DPI presents an interesting problem. The SDL_RenderSetScale function all by itself seems to make almost everything look better on high-DPI displays, and even (mostly) fixes the input alignment issues, but it doesn't do any favors for the fonts.

So I thought the easy/simple solution would be to just oversample the fonts. This does indeed make non-default fonts look better but it also seems that the options which make a certain font look good at low DPI do not necessarily make the same font look good at high DPI even with oversampling. Oversampling makes the font less blurry but to be honest it's still a bit blurry. I also can't seem to get the default font to look pixel-perfect on a high DPI display at all.

Based on the wiki guide, I think it's better to oversample horizontally at 3 times the scale factor instead of exactly at the scale factor, so I'll push that change. Here's what it looks like, first is with high-DPI disabled, second is with high-DPI enabled but with default oversampling, third is with high-DPI enabled but oversampled at 2x2, and last is with high-DPI enabled but oversampled at 6x2:

samples

The last one looks the best to me, but still seems blurry. That might be a problem of texture filtering, though. I tried to use point filtering but the hint doesn't seem to have taken.

@kbolino
Copy link
Contributor Author

kbolino commented Mar 15, 2022

FWIW, I tried some nearby font sizes (13, 13.5, 14, 14.5, 15) but they all looked about equally blurry to me.

@kbolino
Copy link
Contributor Author

kbolino commented Mar 16, 2022

OK, I did some more experiments and learned some things. The blurriness is due to the oversampling. Compare the font atlas (scaled to rendered width) for Droid at size 14 with 6x2 oversampling (left) versus size 28 with 3x1 oversampling (right):

fontatlas

So what I really need is to bake the font at size 2*N but then have it drawn as though it were actually size N. Given that the atlas seems to change in composition with different sizes, I don't see any easy way to do that.

@kbolino
Copy link
Contributor Author

kbolino commented Mar 16, 2022

Well, I got something that works. If you just set the font size to normal_size * scale_y and then mutate

font->handle.height /= scale_y;

after baking the font you get a beautifully scaled font with no blurriness. It even restores the pixel-perfect appearance of the default font (at least, for an integer scale factor). Obviously, this is a hack. How do you think I should proceed?

  1. I can reduce the scope of this PR to just the SDL_RenderSetScale call. This makes the example work out of the box on both regular and high-DPI displays, even though the fonts will look pixelated/blurry on the latter.
  2. I can include this hack, as-is, with a description of what it does and why I did it.
  3. There already exists a better way to do this, and I just couldn't find it. With some guidance, I can do that instead.
  4. There isn't a better way to do this, but you think it's a good idea and should be addressed directly in Nuklear. I can do option (1) for now above and put together a PR for an API change, then follow up with another PR later to amend this demo to use the new API.
  5. Something else entirely

Here's a side-by-side image again, with the excessively oversampled font (left) versus "properly" scaled font (right):

samples2

@dumblob
Copy link
Member

dumblob commented Mar 16, 2022

I for myself would prefer the option (2).

But I'd like to hear some thoughts of others - @Hejsil @aganm @RobLoach @Nielsbishere and anyone who dealt with fonts in Nuklear in the past.

@Nielsbishere
Copy link
Contributor

@dumblob I had the issue too, but I "fixed it" by just enabling MSAA x8 and subpixel rendering. Though I don't think it looked as crisp as the right image. So I think 2 is good yeah

@RobLoach
Copy link
Contributor

For the fonts, I ran into similar issues with raylib-nuklear. Found that if the explicit font size wasn't specified when generated, there would be font scaling issues. Possibly unrelated to the issue here though.

@dumblob
Copy link
Member

dumblob commented Apr 11, 2022

@kbolino it seems everything is as in (2).

If nobody is opposed to this, I'll merge this in a couple of days.

@RobLoach RobLoach merged commit a51d9ee into Immediate-Mode-UI:master Apr 20, 2022
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.

None yet

4 participants