wxMSW: A better (IMHO) RTL fixes for wx{Auto}BufferedDC#26398
Conversation
|
I don't have a real explanation of why we need this commit da9c92e to fix tests failures in |
This comment was marked as low quality.
This comment was marked as low quality.
|
@vslavik I would be grateful if you could test this and see whether it breaks something I’m not aware of. Thanks in advance! |
vadz
left a comment
There was a problem hiding this comment.
Sorry, I'm still very confused by this stuff. It would be great to have an explanation in a comment somewhere about how RTL works in wxMSW, i.e. which coordinates take into account and which don't. Right now I don't understand why is is it correct to disable RTL when converting between device and logical, hopefully it would become clearer with a comment like this...
| // honor the LAYOUT_RTL flag set on the DC, so we have to apply | ||
| // mirroring ourselves (see SetLayout() documentation in MSDN). | ||
| const LONG wDIB = ds.dsBmih.biWidth; | ||
| if ( wDIB > 0 ) |
There was a problem hiding this comment.
Can it really be 0 here? It might be clearer/simpler to return early from this function if the width (or height) is 0.
Sorry, I wanted to, but ran out of time and won’t be able to until May 12-ish. |
b4ba89f to
533b220
Compare
|
I'm still not sure if it this is correct, it seems wrong to disable RTL when doing device/logical conversions. But if this is how the other ports work and if there is no good way to fix this otherwise, we could still merge this... @vslavik Please let me know what do you think when you get back. |
e468462 to
526a148
Compare
|
@AliKet What (if anything) has changed since the previous version after the last force push? |
Nothing interesting, just rebased on master and updating the last commit: > git diff HEAD~1 HEAD
diff --git a/tests/graphics/clippingbox.cpp b/tests/graphics/clippingbox.cpp
index 802f62d29c..bd11ab4871 100644
--- a/tests/graphics/clippingbox.cpp
+++ b/tests/graphics/clippingbox.cpp
@@ -1057,15 +1057,9 @@ static void OneDevRegionRTL(wxDC& dc, const wxBitmap& bmp, bool useTransformMatr
dc.Clear();
// right physical edge becomes left logical edge in a mirrored DC.
const int x2 = s_dcSize.x - (x + w);
- // In a mirrored DC, the device origin (0, 0) is always at the top left
- // of the DC under wxMSW, but under wxGTK3 is at the top right.
-#if defined(__WXGTK3__) || defined(__WXQT__)
+
wxPoint pos = dc.DeviceToLogical(x, y);
wxSize dim = dc.DeviceToLogicalRel(w, h);
-#else
- wxPoint pos = dc.DeviceToLogical(s_dcSize.x-x, y);
- wxSize dim = dc.DeviceToLogicalRel(-w, h);
-#endif
CheckClipBox(dc, bmp,
pos.x, pos.y, dim.x, dim.y,
|
|
Thanks! I'd still like to have some explanation of how RTL works (i.e. what is mirrored and what is not) somewhere, but I'm ready to merge this anyhow. Pinging @vslavik one last time just in case. |
… under wxMSW
Ditto for wxDC::DeviceToLogical{Rel}() functions.
In RTL layout, these functions always return unmirrored coordinates under wxGTK3
and wxQt. This is now the case under wxMSW too for consistency and also to fix
double-mirroring problem when passing the transformed coordinates to GDI drawing
functions. See wxGrid::Render() for example.
Also revert commit 3d40c2a (Fix wxDC::StretchBlit() for non-0 xsrc in RTL layout) which is no longer needed.
This constant is only used inside ApplyEffectiveScale() No real changes.
Zooming and scaling the GDI device context in RTL layout was already broken even before the latest changes. - Try enabling (Zoom 125%) under (File > Render setup) before clicking on (Render) in the griddemo. - Try Scale (Ctrl+H) in the drawing sample. The implementation has been changed to use the GDI function ScaleWindowExtEx() instead, which gives better results even in an LTR layout.
Also fix custom drawing where
wxDC::DeviceToLogical()and related functions need to be used.