Skip to content

Conversation

@goderbauer
Copy link
Member

Description

Before this change a RenderPointerListener that had active hover listeners (onPointerEnter, onPointerHover, onPointerExit) would drop any previous transforms on the floor if nobody else in that subtree required compositing. With this change RenderPointerListener itself now requests compositing if one of the hover listeners is set.

Debugging notes: If Nobody in a subtree requests compositing, the RenderTransform will execute the provided transform directly on the canvas instead of introducing a TransformLayer. When the RenderPointerListener pushes a new AnnotatedRegionLayer to make hover detection work, a new canvas for a new PictureLayer is created and all subsequent render objects draw into this new canvas, which doesn't know anything about the transform. Hence, the transform is ignored for all following draw operations.
If you push a layer, you need to request compositing to ensure that everything works correctly. Ideally, we should have an assert somewhere ensuring this.

Related Issues

Fixes #31986.

Tests

I added the following tests:

  • A test to verify that RenderPointerListener with active hover listeners doesn't drop the transforms on the floor.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

Copy link
Contributor

@Hixie Hixie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should also test what happens when you change the state of the bit

@goderbauer goderbauer force-pushed the hoverwithtransform branch from 1ad5e13 to 496a86b Compare May 6, 2019 09:15
@goderbauer
Copy link
Member Author

Test added and rebased.

@goderbauer goderbauer added framework flutter/packages/flutter repository. See also f: labels. waiting for tree to go green labels May 6, 2019
@goderbauer goderbauer merged commit cc23958 into flutter:master May 6, 2019
@goderbauer goderbauer deleted the hoverwithtransform branch May 6, 2019 11:36
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hover Listener ignores any previous transforms

4 participants