Opened 8 months ago

Closed 8 months ago

#15734 closed defect (fixed)

wxRichTextCtrl: A floating wxRichTextTable's cells aren't drawn

Reported by: dghart Owned by: juliansmart
Priority: normal Milestone:
Component: wxRichText Version: dev-latest
Keywords: wxRichTextTable float range Cc:
Blocked By: Blocking:
Patch: yes

Description

When a wxRichTextTable floats, all (or all-but-one) of its cells are not drawn. This can be seen in the richtext sample: just give the table either float property. Presumably this happens because a table's cells aren't held in a paragraph, and so aren't collected by the wxRichTextFloatCollector; though even if they were, I suspect it wouldn't work correctly.

I don't know how this should be fixed. Ideally (at least for tables) it would happen at the Layout() stage, but afaict it's delayed until Draw(). Failing that, wxRichTextFloatCollector should take contained objects into account. Least ideal, but perhaps simplest, a object's floating position could be cached in the object and so be available to its contents; I can't see that this is possible at present.

Attachments (1)

richtextbuffer.patch download (616 bytes) - added by dghart 8 months ago.

Download all attachments as: .zip

Change History (4)

comment:1 follow-up: Changed 8 months ago by juliansmart

  • Status changed from new to confirmed

Hi David,

This also happens with boxes. I don't think it's related to table cells not being a paragraph, because the float collector would only collect objects with the floating attribute set, i.e. the top-level table or box which is in a paragraph. Also I'm not seeing float collection/layout being delayed until Draw.

I suspect it may be related to this comment in wxRichTextParagraph::LayoutFloat:

anchored->Move(wxPoint(x, pos)); should move children

I think the comment means it does move the children, but for some reason it's not working correctly for composite objects (the code was originally written as a Google Summer of Code project with images in mind).

I suspect if we fix it for boxes, it'll probably work for tables as well.

I will look at it more closely as soon as I can.

Thanks,

Julian

comment:2 in reply to: ↑ 1 Changed 8 months ago by dghart

  • Keywords range added
  • Patch set

Replying to juliansmart:

I suspect it may be related to this comment in wxRichTextParagraph::LayoutFloat:

anchored->Move(wxPoint(x, pos)); should move children

A good thought, but it seems that Move() works correctly.

However debugging this did lead me to the cause of the problem: when Draw() is called for a top-level object, the 'range' parameter is the object's own range. Except that, for floating objects, it's not; GetRange() is used, so any contents will almost always be outside the given range. That also explains why one cell gets drawn when the table is near the beginning of the textctrl: it just happens to be within range.

The attached patch corrects this in wxRichTextFloatCollector::DrawFloat. It fixes my use-case, and probably 'richtext' too, though the drawing artifacts mentioned in #15286 make it difficult to be sure.

Changed 8 months ago by dghart

comment:3 Changed 8 months ago by juliansmart

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

Yay! Good catch. I've applied to fix to both branches.

Note: See TracTickets for help on using tickets.