Opened 6 months ago

Closed 2 months ago

Last modified 2 months ago

#16250 closed defect (fixed)

wxGrid problems in RTL layout

Reported by: benadams Owned by: VZ
Priority: normal Milestone: 3.1.0
Component: wxGrid Version: dev-latest
Keywords: DC RTL Cc:
Blocked By: Blocking:
Patch: yes

Description

I'm using wxpython 3.0.0.0 on Windows XP.
I work with RTL i.e:
self.SetLayoutDirection(wx.Layout_RightToLeft)

When try to scrolling CSheet(or Grid) horizontally the grid Col labels don't draw properly. If I try to make changes to the grid this causes further drawing issues on the column labels.

attache example file.

Attachments (8)

bug.py download (549 bytes) - added by benadams 6 months ago.
bug code example
grid-rtl-1.png download (38.5 KB) - added by awi 3 months ago.
RTL grid scrolled horizontally.
grid-rtl-2.png download (32.2 KB) - added by awi 3 months ago.
RTL grid scrolled vertically.
Fix-drawing-column-labels-in-RTL-mode.patch download (585 bytes) - added by awi 2 months ago.
Fix drawing column labels.
Fix-erasing-cells-in-RTL-mode.patch download (640 bytes) - added by awi 2 months ago.
Fix erasing cell content.
Fix-dragging-columns-in-RTL-mode.patch download (975 bytes) - added by awi 2 months ago.
Fix dragging columns.
Fix-drawing-rectangle-in-RTL-mode.patch download (1.3 KB) - added by awi 2 months ago.
Fix drawing rectangle in RTL mode.
Remove-unnecessary-references-to-GetLayoutDirection.patch download (1.3 KB) - added by awi 2 months ago.
Remove manual mirroring.

Download all attachments as: .zip

Change History (20)

Changed 6 months ago by benadams

bug code example

comment:1 Changed 6 months ago by vadz

  • Milestone 3.0.1 deleted
  • Priority changed from high to normal
  • Summary changed from problem with creating a RTL CSheet\wxGrid to wxGrid problems in RTL layout

comment:2 Changed 3 months ago by awi

  • Keywords DC added
  • Version changed from 3.0.0 to dev-latest

This issue can be reproduced with latest trunk (r76995) and griddemo sample:

  • samples/grid/griddemo.cpp

    a b GridFrame::GridFrame() 
    243243                   wxDefaultSize ) 
    244244{ 
    245245    SetIcon(wxICON(sample)); 
     246    SetLayoutDirection(wxLayout_RightToLeft); 
    246247 
    247248    wxMenu *fileMenu = new wxMenu; 
    248249    fileMenu->Append( ID_VTABLE, wxT("&Virtual table test\tCtrl-V")); 

When grid is scrolled horizontally and vertically then several strange visual effects appear.
RTL grid scrolled horizontally.
RTL grid scrolled vertically.

After some initial testing I would say that this is issue is not related to wxGrid itself but rather to DC objects. wxGrid renderers use wxPaintDC and wxClientDC which presumably don't work as expected in RTL mode.
I think this issue can be related to #16254.

Changed 3 months ago by awi

RTL grid scrolled horizontally.

Changed 3 months ago by awi

RTL grid scrolled vertically.

comment:3 Changed 3 months ago by awi

  • Status changed from new to confirmed

comment:4 Changed 2 months ago by awi

  • Patch set

Unfortunately, I was too optimistic claiming that only wxDC can be blamed for these issues.
There are some minor glitches in wxGrid which cause some strange visual effects under WXMSW.

  1. Column labels are not displayed correctly because device origin is manually mirrored and this is not necessary.

attachment:Fix-drawing-column-labels-in-RTL-mode.patch

  1. Artifacts near vertical grid lines appear because not the entire cell area is erased at redrawing. Rectangle Win API can be blamed for this issue because it apparently doesn't work the same way in RTL and LTR mode.

attachment:Fix-erasing-cells-in-RTL-mode.patch

And one unrelated issue with dragging columns. Currently, in RTL mode there is not determined correctly if mouse pointer is over left/right (near/far) half of the column which leads to strange final effects (columns are moved to wrong locations).
In this case there also not necessary to manually mirror mouse pointer position relative to the column location.
attachment:Fix-dragging-columns-in-RTL-mode.patch

Changed 2 months ago by awi

Fix drawing column labels.

Changed 2 months ago by awi

Fix erasing cell content.

Changed 2 months ago by awi

Fix dragging columns.

comment:5 Changed 2 months ago by awi

I reanalyzed issue no. 2 (erasing cells) and I think that the problem is indeed about how Rectangle API is used. There stated in MSDN that rectangle is drawn without bottom and right edges. They said "right" not "far" and I think this edge description should be taken literally.
Currently, always x2 coordinate (far) is adjusted in order to draw whole rectangle (in wxMSWDCImpl::DoDrawRectangle). This is OK in LTR mode because far edge is displayed as a right one.
I think layout direction should be taken here into account and in RTL mode x1 coordinate (near, but displayed as right) should be adjusted, not x2.
Patch fixing this issue is attached (attachment:Fix-drawing-rectangle-in-RTL-mode.patch).

This of course fixes issue no. 2.

Changed 2 months ago by awi

Fix drawing rectangle in RTL mode.

comment:6 Changed 2 months ago by vadz

  • Keywords RTL added
  • Milestone set to 3.1.0

Thanks again for continuing to work on this!

I believe manual mirroring should be just as unnecessary in the other ports as in wxMSW, the code manually calling GetLayoutDirection() probably predates inheriting of the layout direction from the window (although I have some doubts about r64475, it was definitely recent enough to already have the correct DC layout, yet still introduced this check... But I think it was still a mistake). So I believe all checks for GetLayoutDirection() should be just removed.

I also think that DrawRectangle() workaround belongs to wxDC itself, we can't expect all the code using it to account for the RTL manually like this. So unless this is something really specific to wxGrid, could we please do it there?

TIA!

comment:7 Changed 2 months ago by vadz

P.S. Our comments crossed, you've already taken care of the second one, thanks!

comment:8 Changed 2 months ago by VZ

In 77027:

Fix wxDC::DrawRectangle() when using RTL in wxMSW.

Extend the correct edge of the rectangle (always the physical right, not the
logical right) to fix off by one errors in RTL mode, affecting notably wxGrid.

See #16250.

comment:9 Changed 2 months ago by VZ

In 77028:

Fix wxDC::DrawRectangle() when using RTL in wxMSW.

Extend the correct edge of the rectangle (always the physical right, not the
logical right) to fix off by one errors in RTL mode, affecting notably wxGrid.

See #16250.

comment:10 Changed 2 months ago by awi

Patch removing code to manual mirroring is attached.

Changed 2 months ago by awi

Remove manual mirroring.

comment:11 Changed 2 months ago by VZ

  • Owner set to VZ
  • Resolution set to fixed
  • Status changed from confirmed to closed

In 77037:

Remove manual mirroring in RTL case from wxGrid code.

This is not necessary as wxDC already inherits RTL from the window itself and,
in fact, breaks the display when using RTL.

Closes #16250.

comment:12 Changed 2 months ago by VZ

In 77038:

Remove manual mirroring in RTL case from wxGrid code.

This is not necessary as wxDC already inherits RTL from the window itself and,
in fact, breaks the display when using RTL.

Closes #16250.

Note: See TracTickets for help on using tickets.