This is a brain dump of a potential problem I may have discovered in our text rendering.
I was looking through our text rendering code and I was looking at TextFrame::SetPerFrameData. This function is called in LazyGlyphAtlas::AddTextFrame, and that happens in FirstPassDispatcher::DrawText, which happens on the raster thread right before we render a displaylist to a target.
The frame data includes the matrix transform of the text, which is not actually specific to the text frame, but specific to that draw command of that text frame, so it seems wrong to attach it to the text frame itself. In addition, it seems at this point in the pipeline, the text frame itself being mutated feels a little wrong. I can imagine a scenario that leads wrong behavior:
- Create a picture recorder
- Draw a paragraph to that picture recorder and end recording, yielding a picture
- Add this picture to multiple places in a single display list tree that have different matrix transforms on them
- When we render this display list tree, the first pass dispatcher will run, and each time it encounters this picture it will set the matrix transform on the text frame based on the current transform, overwriting it multiple times.
- When we actually get to rendering this info and populating the glyph atlas, it will use the last written transform for all of the instances of that text frame in the tree. (Which is wrong).
This might be a contrived example, but it sort of illustrates why this data shouldn't be mutated on the text frame itself. Ideally, the information attached to the text frame itself should be agnostic to the surrounding drawing context, and the lazy glyph atlas can store this extra information either in a wrapper object or in a sibling object to the text frame.
This is a brain dump of a potential problem I may have discovered in our text rendering.
I was looking through our text rendering code and I was looking at
TextFrame::SetPerFrameData. This function is called inLazyGlyphAtlas::AddTextFrame, and that happens inFirstPassDispatcher::DrawText, which happens on the raster thread right before we render a displaylist to a target.The frame data includes the matrix transform of the text, which is not actually specific to the text frame, but specific to that draw command of that text frame, so it seems wrong to attach it to the text frame itself. In addition, it seems at this point in the pipeline, the text frame itself being mutated feels a little wrong. I can imagine a scenario that leads wrong behavior:
This might be a contrived example, but it sort of illustrates why this data shouldn't be mutated on the text frame itself. Ideally, the information attached to the text frame itself should be agnostic to the surrounding drawing context, and the lazy glyph atlas can store this extra information either in a wrapper object or in a sibling object to the text frame.