Msw rtl2 (PR #25426)
AliKet
@vadz @a-wi I'm just wondering why this commit is needed? because LPtoDP/DPtoLP should work correctly in RTL layout as documented.
Anyway, currently wxMSW is broken in RTL layout:
- Try scrolling
wxGrid(in griddemo) - Try switching to Regions screen in drawing sample to see the problem.
I tested this PR in RTL layout and it works correctly for me. Although running the RTL tests still fail and I think I should disable them for now untill I (or anyone else) manage to run them successfully in the future!
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/25426
Commit Summary
- bf1b4a3 Revert commit 288b208a0a4d306afa7c356af6e17105bd24485c
- 754ed54 Remove a hack from wxPaintDC
- 42c520b wxMSW: Fix SetClippingRegion() after reverting commit 288b208a0a
- 928d7eb wxMSW: Workaround to fix rendering wxWidgets in RTL layout
File Changes
(3 files)
- M include/wx/msw/dc.h (2)
- M src/msw/dc.cpp (80)
- M src/msw/dcclient.cpp (4)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/25426.patch
- https://github.com/wxWidgets/wxWidgets/pull/25426.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- 1d027cb Disable RTL tests failing under wxMSW
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25426)Sorry, I don't have time to look at this right now, but it seems wrong to disable the previously working tests unless we have good reasons to think the tests themselves were incorrect -- do we?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25426)Sorry, I don't have time to look at this right now, but it seems wrong to disable the previously working tests unless we have good reasons to think the tests themselves were incorrect -- do we?
I'll try to look into the failing tests later and try to fix them, but RTL in master is definitely broken! see:
drawing_rtl_bad.png (view on web)
grid_rtl_bad.png (view on web)
With this PR:
drawing_rtl_ok.png (view on web)
grid_rtl_ok.png (view on web)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25426)Yes, it does look broken, but I'm worried about partially but not completely fixing it in 3.3.0, resulting in a different kind of brokenness there and more fixes needed for 3.3.1.
The problem is really that we plan to release it in 2 days and this is a relatively big change and I (don't know about you?) am far from being 100% confident that it's correct...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25426)Yes, it does look broken, but I'm worried about partially but not completely fixing it in 3.3.0, resulting in a different kind of brokenness there and more fixes needed for 3.3.1.
The problem is really that we plan to release it in 2 days and this is a relatively big change and I (don't know about you?) am far from being 100% confident that it's correct...
It's not urgent for me for this to be included in the upcoming release, and I plan to debug this later to try to get rid of commit 928d7eb if possible because it really bothers me. So I suggest adding work neededto this as a reminder, TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 2 commits.
- 508460d wxMSW: Fix text rendering in a transformed DC in RTL layout
- 18351af Disable RTL tests failing under wxMSW
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25426)drawing sample with and without commit 508460d and a transformation applied to the DC:
With:
drawing_trans_rtl_bad.png (view on web)
Without:
drawing_trans_rtl_ok_2.png (view on web)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25426)I'd really like to merge this but I feel that if we do it now, we'll never reenable the tests later, so let's postpone it a bit until someone has time to understand what is broken here.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 8 commits.
- 4b0a21e Revert commit 288b208a0a4d306afa7c356af6e17105bd24485c
- 36ccacc Remove a hack from wxPaintDC
- 6e2c6c3 wxMSW: Fix SetClippingRegion() after reverting commit 288b208a0a
- 5dac745 wxMSW: Fix text rendering in a transformed DC in RTL layout
- 1d2773b Fix clippingbox test in RTL layout
- 9506e98 wxGTK: Fix SetLayoutDirection() not working with wxMemoryDC
- 1ba8257 Fix wxGCDC::DeviceToLogical() in RTL layout where the DC is mirrored
- 1a3b5e0 wxGTK: Enable disabled tests in RTL layout
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 6 commits.
- c74907c wxMSW: Fix text rendering in RTL layout with a transformed DC
- 4f49395 Fix clippingbox's OneDevRegionRTL test case
- 52eade9 wxGTK: Fix SetLayoutDirection() not working with wxMemoryDC
- 76b64e0 Fix wxGCDC::DeviceToLogical() not working correctly in RTL layout
- 403e9f2 wxGTK: Enable disabled tests in RTL layout
- 1388896 Fix an off-by-one error in OneDevRegionRTL test case
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- c5e2cae Fix an off-by-one error in OneDevRegionRTL test case
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 2 commits.
- 6e94053 wxGTK: Fix ScreenToClient() returning incorrect results in RTL layout
- ee9c8e0 wxAUI: Don't mirror hint rectangle in RTL layout
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25426)@vadz @paulcor Is this commit 6e94053 looks correct to you? Because it works correctly for me with wxGTK3 in RTL layout (try moving panes in the auidemo).
Disclaimer: I haven't tested it under wxGTK2 because last time I checked the RTL support wasn't great there!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 4 commits.
- 60d32b6 wxGTK: Fix popup menu's position in RTL layout
- 19f2b11 wxGTK: Fix mouse position when initializing wxMouseEvent in RTL layout
- 07acfbf Add tests to wxWindow::ClientToScreen() for RTL layout
- c5d3e44 wxGTK: Fix wxDataViewCtrl rendering in RTL layout
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25426)@vadz @paulcor I think there is a bug in wxDataViewCheckIconTextRenderer() when using the native wxDataViewCtrl control in wxGTK3 (I'm not sure because I'm not familiar with the (native) wxDataViewCtrl drawing code). i.e.: the check+icon+text is always drawn LTR. The generic one (compiled with --disable-nativedvc) works correctly though with these changes.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25426)@vadz Sorry this PR is no longer wxMSW port specific!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 15 commits.
- 32a3e8f Revert commit 288b208a0a4d306afa7c356af6e17105bd24485c
- 5f3b21b Remove a hack from wxPaintDC
- 4d2ff42 wxMSW: Fix SetClippingRegion() after reverting commit 288b208a0a
- 0dca212 wxMSW: Fix text rendering with a transformed DC in RTL layout
- 48da250 Fix OneDevRegionRTL test case in tests/graphics/clippingbox.cpp
- 60a5d1f wxGTK: Fix SetLayoutDirection() not working with wxMemoryDC
- 1f67f65 Fix wxGCDC::DeviceToLogical() not working correctly in RTL layout
- 8233473 wxGTK: Enable disabled tests in RTL layout
- 00c0ebe Fix an off-by-one error in OneDevRegionRTL test case
- efdc6bb wxGTK: Fix ScreenToClient() returning incorrect results in RTL layout
- 98169ba wxAUI: Don't mirror hint rectangle in RTL layout
- b3b385b wxGTK: Fix popup menu's position in RTL layout
- 2099450 wxGTK: Fix mouse position when initializing wxMouseEvent in RTL layout
- 31628a7 Add tests to wxWindow::ClientToScreen() for RTL layout
- de29b44 wxGTK: Fix wxDataViewCtrl rendering in RTL layout
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 11 commits.
- 0387253 wxMSW: Revert commit 288b208a0a4d306afa7c356af6e17105bd24485c
- 30de8f1 wxMSW: Remove a hack from wxPaintDC
- f5d326f wxMSW: Fix SetClippingRegion() after reverting commit 288b208a0a
- 8c8927d wxGTK: Fix SetLayoutDirection() not working with wxMemoryDC
- 324f030 wxGTK: Fix ScreenToClient() returning incorrect results in RTL layout
- 6d692d0 wxGTK: Fix popup menu's position in RTL layout
- ed15a49 wxGTK: Fix mouse position when initializing wxMouseEvent in RTL layout
- 53dda62 wxGTK: Fix wxDataViewCtrl rendering in RTL layout
- 27b95ab wxAUI: Don't mirror hint rectangle in RTL layout
- 68c2b98 Fix OneDevRegionRTL test case after the changes done to wxMSW and wxGTK3
- 9e1e58f Add tests to wxWindow::ClientToScreen() for RTL layout
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 4 commits.
- d547521 wxGTK: Fix wxDataViewCustomRenderer in RTL layout
- cc6c84f wxAUI: Don't mirror hint rectangle in RTL layout
- 32988e4 Fix OneDevRegionRTL test case after the changes done to wxMSW and wxGTK3
- de883a0 Add tests to wxWindow::ClientToScreen() for RTL layout
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
AliKet left a comment (wxWidgets/wxWidgets#25426)@vadz @paulcor I think there is a bug in
wxDataViewCheckIconTextRenderer()when using the nativewxDataViewCtrlcontrol inwxGTK3(I'm not sure because I'm not familiar with the (native)wxDataViewCtrldrawing code). i.e.: the check+icon+text is always drawn LTR. The generic one (compiled with--disable-nativedvc) works correctly though with these changes.
Fixed in d547521
Before:
dataview_rtl_bad.png (view on web)
After:
dataview_rtl_ok.png (view on web)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
AliKet
@AliKet pushed 1 commit.
- 143c318 Add tests to wxWindow::ClientToScreen() for RTL layout
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz approved this pull request.
This looks very nice, I really appreciate getting rid of the hacks in wxMSW and platform checks in the tests, thanks a lot for your work on this!
I'll merge this soon, if you agree that we should use GetConnectWidget(), we can/will change this later.
In include/wx/gtk/private/event.h:
> {
// origin in the upper right corner
GtkAllocation a;
- gtk_widget_get_allocation(win->m_wxwindow, &a);
+ gtk_widget_get_allocation(win->m_wxwindow ? win->m_wxwindow : win->m_widget, &a);
I wonder if we should do this here:
⬇️ Suggested change- gtk_widget_get_allocation(win->m_wxwindow ? win->m_wxwindow : win->m_widget, &a); + gtk_widget_get_allocation(win->GetConnectWidget(), &a);
?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Merged #25426 into master.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.