Skip to content

Add HiDPI support for the Qt toolkit#591

Merged
jwiggins merged 7 commits into
masterfrom
feature/hi-dpi-qt5
Feb 19, 2021
Merged

Add HiDPI support for the Qt toolkit#591
jwiggins merged 7 commits into
masterfrom
feature/hi-dpi-qt5

Conversation

@jwiggins
Copy link
Copy Markdown
Member

@jwiggins jwiggins commented Feb 16, 2021

This is the Qt half of #412 and #517. I haven't looked into how Wx does HiDPI display yet, but figuring out how to do it for Qt5 first will likely make adding Wx "easy".

Known broken:

  • Component.use_backbuffer creates a low resolution buffer.
  • OpenGL clipping is a bit off
  • GraphicsContext.clip_to_rect uses absolute coordinates, so supporting HiDPI by changing the scale of a GC will break lots of code [but not all] which uses clip_to_rect GraphicsContext.clip_to_rect is not uniformly implemented with respect to the affine transformation across all kiva backends! (Celiagg's clip_to_rect() should respect the transform matrix #592)
  • GraphicsContext.draw_marker_at_points in the kiva.agg backend will draw nothing if there's anything more than a translation applied to the transformation matrix. Covered by Allow scaling in agg's draw_marker_at_points #594

@jwiggins jwiggins marked this pull request as ready for review February 18, 2021 11:02
@corranwebster
Copy link
Copy Markdown
Contributor

This looks generally fine in terms of implementation. I haven't taken it for a spin yet, so there may be some glitches out there still.

There are some edge-cases that I can see around what happens if the resolution changes while the window is live (either by the user changing the resolution, or moving between screens), but as-is should be no worse than what we have now. I think it can be supported in Qt if we wanted to go there, but worst case is that you get low-resolution images after moving from low-res to high-res; or you render at high-res when working with low-res.

This PR should include some tests: testing the Kiva GCs should be straightforward, but testing that the resolution is set correctly in CI is probably going to need a mock/patch of the appropriate Qt methods - I have no idea what headless boxes are going to report as screen resolution.

@corranwebster
Copy link
Copy Markdown
Contributor

Just had a thought where there may be additional breakage: in save methods we probably need to adjust the dpi appropriately. Pillow will take the raw data just fine, but won't pass the scale information through to saved files.

@jwiggins
Copy link
Copy Markdown
Member Author

I've added some modifications to the kiva drawing tests which exercise the pixel scaling code.

@jwiggins
Copy link
Copy Markdown
Member Author

Shakes fist at OpenGL backend 😠

@jwiggins
Copy link
Copy Markdown
Member Author

#593 added to track saved image DPI concerns.

@corranwebster
Copy link
Copy Markdown
Contributor

Playing around with this a little, it seems to be working for the QPainter backend, but I think #390 might be impacting the drawing in the Agg backend - a little hard to tell as the number of pixels a line requires depends on the fractional position of the line - or alternatively it may be that we are snapping lines to coordinates somewhere (I have a vague recollection of such code).

@jwiggins
Copy link
Copy Markdown
Member Author

#390 is definitely a factor for the AGG backends. They're definitely implemented incorrectly, but I can only imagine how to fix celiagg and not how to fix kiva.agg.

@jwiggins jwiggins merged commit 25a5abc into master Feb 19, 2021
@jwiggins jwiggins deleted the feature/hi-dpi-qt5 branch February 19, 2021 10:51
@jwiggins
Copy link
Copy Markdown
Member Author

Thanks for the testing and feedback.

jwiggins added a commit that referenced this pull request Feb 19, 2021
HiDPI rendering for Qt 5+.

The change is mostly transparent to user code, because kiva drawing is already based on vectors.
However, users can opt out by passing `high_resolution=False` to `ComponentEditor`.
jwiggins added a commit that referenced this pull request Feb 19, 2021
HiDPI rendering for Qt 5+.

The change is mostly transparent to user code, because kiva drawing is already based on vectors.
However, users can opt out by passing `high_resolution=False` to `ComponentEditor`.
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.

3 participants