Skip to content

[impeller] TextFrame::SetPerFrameData is weird and possibly buggy #177424

Description

@eyebrowsoffire

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Important issues not at the top of the work liste: impellerImpeller rendering backend issues and features requestsengineflutter/engine related. See also e: labels.team-engineOwned by Engine teamtriaged-engineTriaged by Engine team

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions