Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@flar
Copy link
Contributor

@flar flar commented Feb 13, 2022

Fixes dnfield/flutter_svg#661
(Note that the first example in that issue was the only issue that I was able to see a problem on, the other 3 issues added as github comments don't seem to fail for me. But, this fix does fix that first example.)

The problem causing the SVG to not display was the fact that we were getting the bounds of nested save/saveLayer calls wrong. At some point I decided to accumulate the bounds of the layers into their own accumulators and then transforming and clipping them when I accumulated those bounds into the outer layer's bounds in the restore() call. Unfortunately, this doesn't work because the clip is in device pixels (actually in "the pixel coordinate space in which the DL is rendered" to be specific) and so when I reset the matrix and not the clip I would end up with mismatched a clip and matrix that didn't agree on the coordinate space in which they applied.

I fixed this by 2 essential changes:

  • I no longer reset the matrix or the clip for a new layer, all operations accumulate in DL-root-coordinates at all times
  • I got rid of the spaghetti tree of LayerData subclasses in favor of a single class that just remembers the needed information and lets the restore() call do the work in one location that is easier to understand and maintain.

@zanderso
Copy link
Member

How hard would it be to backport this to the 2.10 branch here https://github.com/flutter/engine/commits/flutter-2.8-candidate.16 ? (Ignore that the branch name says 2.8 in it, it's really the 2.10 branch.)

@flar flar added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Feb 13, 2022
@flar
Copy link
Contributor Author

flar commented Feb 13, 2022

How hard would it be to backport this to the 2.10 branch here https://github.com/flutter/engine/commits/flutter-2.8-candidate.16 ? (Ignore that the branch name says 2.8 in it, it's really the 2.10 branch.)

It isn't a clean cherry-pick, but the changes are simple. Should I create a PR?

I created #31440 as a proof of concept for cherry-picking.

@zanderso
Copy link
Member

I created #31440 as a proof of concept for cherry-picking.

Perfect. The luci-engine check is red, but the tree is actually green (/cc @godofredoc), so feel free to land this whenever you're ready.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some SVG stop displaying with Flutter 2.10.0

2 participants