Opened 5 years ago

Closed 3 months ago

Last modified 3 months ago

#17391 closed defect (fixed)

OpenGL cube sample doesn't look right with DPI scaling under GTK

Reported by: vadz Owned by: Vadim Zeitlin <vadim@…>
Priority: low Milestone: 3.1.4
Component: wxGTK Version: dev-latest
Keywords: OpenGL HiDPI Cc: mmarsan@…, john.j.beard@…, swt@…, stoffball@…
Blocked By: Blocking:
Patch: yes

Description

When using wxGTK3 on high DPI (scale factor 2) screen, the cube appears in the lower left window quadrant only.

I'm not sure if it's a bug in the sample or wxGLCanvas itself but it would be nice to fix this.

Attachments (4)

gdk_scale-1.png download (11.9 KB) - added by jjbeard 19 months ago.
gdk_scale-2.png download (14.8 KB) - added by jjbeard 19 months ago.
opengl_samples_win.patch download (2.7 KB) - added by tm 11 months ago.
Patch for Windows
opengl_scale.patch download (7.1 KB) - added by tm 11 months ago.
Implemtation of GetOpenGLScaleFactor()

Download all attachments as: .zip

Change History (16)

Changed 19 months ago by jjbeard

Changed 19 months ago by jjbeard

comment:1 Changed 19 months ago by jjbeard

  • Cc john.j.beard@… added

I can still produce this on WX head on GTK+3 (v3.1.2-567-gcfded96627), even without a hiDPI monitor, by manipulating the GDK_SCALE environment variable.

Running the cube demo with GDK_SCALE=1 works fine, using GDK_SCALE=2 produces the cube in only the lower left quarter.

This affects KiCAD on hiDPI displays, as the OpenGL drawing canvas is subject to the same bug (https://bugs.launchpad.net/kicad/+bug/1797308).

Last edited 19 months ago by jjbeard (previous) (diff)

comment:2 Changed 19 months ago by jjbeard

  • Cc john.j.beard@… removed

It looks like this is the issue (adapted from cube.cpp:352):

    const wxSize ClientSize = GetClientSize();
    <snip>
    glViewport(0, 0, ClientSize.x, ClientSize.y);

Changing this to:

    const wxSize ClientSize = GetClientSize() * GetContentScaleFactor();
    glViewport(0, 0, ClientSize.x, ClientSize.y);

Fixes both GDK_SCALE=2 and GDK_SCALE=1 remains the same.

Is this the correct approach here?

comment:3 Changed 19 months ago by mmarsan

  • Cc mmarsan@… added

OpenGL driver normally uses DRI (Direct Rendering Infrastructure). This means glViewPort uses physical sizes of the window and is unaware of OS scaling stuff, just because OS scaling is not used by OGL.

While VZ's solution works, and likely is the way to go, I think some clarifications are needed in the wx docs about physical/scaled/logical/virtual and units used by all window size functions. I'm thinking right now about GetClient|Virtual|Max|Best|...|Size(), GetContentScaleFactor(), From|ToDPI(). And wxDialog units (this is mostly clear).

Perhaps a new function, say GetPhysicalSize may be useful.

comment:4 Changed 19 months ago by jjbeard

  • Cc john.j.beard@… added

comment:5 Changed 15 months ago by swt2c

  • Cc swt@… added

comment:6 Changed 13 months ago by Vadim Zeitlin <vadim@…>

  • Owner set to Vadim Zeitlin <vadim@…>
  • Resolution set to fixed
  • Status changed from new to closed

In b134589cb/git-wxWidgets:

Fix OpenGL samples when using HiDPI displays

OpenGL seems to operate using physical pixels, so we need to factor in the
scale factor when setting the GL viewport.

Closes #17391.

Closes https://github.com/wxWidgets/wxWidgets/pull/1470

Changed 11 months ago by tm

Patch for Windows

comment:7 Changed 11 months ago by tm

  • Resolution fixed deleted
  • Status changed from closed to reopened

With the applied patch the OpenGL samples now look cropped on a HiDPI display and Windows. (Tested with wxWidgets git version, compiler MSVC 2017 64 bit and Windows 10).
According to docs/Changes.txt the multiplication with GetContentScaleFactor() should only be done for wxGTK3 and wxOSX.
I've uploaded a patch with these changes.

comment:8 Changed 11 months ago by vadz

  • Milestone set to 3.1.4

Thanks for reporting the problem, but I don't think it's the right solution, we don't want to tell people to use platform checks in their code.

I'm not sure what do we want to do here, however. Add some GetOpenGLScaleFactor() that would be implemented differently depending on the platform?

Changed 11 months ago by tm

Implemtation of GetOpenGLScaleFactor()

comment:9 Changed 11 months ago by tm

  • Cc stoffball@… added

I'm attaching a patch which implements the proposed GetOpenGLScaleFactor() version. The samples have been updated accordingly.
(I tested only on Windows, but tried to add the code for GTK3 and OSX.)

comment:10 Changed 3 months ago by vadz

  • Patch set

Thanks for your patch! I've finally made this PR with it and will merge it once it passes.

BTW, please let me know (here or via private email) if you'd like to be credited for this (and your previous) contributions using your real name/email rather than just tm <tm@trac.wxwidgets.org>.

comment:11 Changed 3 months ago by Vadim Zeitlin <vadim@…>

  • Resolution set to fixed
  • Status changed from reopened to closed

In f6cc8ff5/git-wxWidgets:

Add GetOpenGLScaleFactor() to abstract OpenGL coordinates scaling

The fix for OpenGL coordinates when using high DPI in b134589cbb (Fix
OpenGL samples when using HiDPI displays, 2019-08-06) did fix it for GTK
3 and macOS, but broke it for MSW and other platforms not using pixel
scaling, as window coordinates are the same as OpenGL ones there, while
GetContentScaleFactor() can still return values > 1 even on these
platforms.

Provide new GetOpenGLScaleFactor() function abstracting this platform
difference and use it in all OpenGL samples to make them work correctly
in high DPI under all platforms.

Closes https://github.com/wxWidgets/wxWidgets/pull/1944

Closes #17391.

comment:12 Changed 3 months ago by Vadim Zeitlin <vadim@…>

In 379e718a/git-wxWidgets:

Remove recently added GetOpenGLScaleFactor()

It has become unnecessary after the previous commit, as now the generic
GetContentScaleFactor() can be used instead of it on all platforms, so
revert the changes of f6cc8ff52c (Add GetOpenGLScaleFactor() to abstract
OpenGL coordinates scaling, 2020-07-10).

See https://github.com/wxWidgets/wxWidgets/pull/1944

See #17391.

Note: See TracTickets for help on using tickets.