-
Notifications
You must be signed in to change notification settings - Fork 29.7k
InteractiveViewer.builder #77414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
InteractiveViewer.builder #77414
Conversation
| return Offset.zero & parentRenderBox.size; | ||
| } | ||
| } | ||
| return Offset.zero & _maxConstraints; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was previously using _parentKey to get the size of the viewport, but now I need it before the first render. It seems like using LayoutBuilder's max constraints should work just as well... Is there any case where that won't work?
By "viewport" I mean the size of the InteractiveViewer widget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable. Instead of storing it in a field, can you just turn this into a method and pass it in from the build method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I've realized that the constraints-based calculation will be wrong when InteractiveViewer is sized by its child, i.e. when the child is smaller than the constraints. This is the typical problem of knowing the size one frame too late.
I think I've worked around it by requiring InteractiveViewer.builder to be used with constrained: false. That will make this constraints-based viewport calculation correct, and I don't think there's a use case for needing constrained to be true with builder.
If you've got another idea let me know, though.
| child: child, | ||
| child: LayoutBuilder( | ||
| builder: (BuildContext context, BoxConstraints constraints) { | ||
| _maxConstraints = Size(constraints.maxWidth, constraints.maxHeight); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2cd0cf5 to
c382c11
Compare
|
@goderbauer Ready for re-review. I'm using this in my talk and it seems to work well. |
goderbauer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| /// Create an InteractiveViewer. | ||
| /// | ||
| /// The [child] parameter must not be null. | ||
| /// The child parameter must not be null. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
| /// The child parameter must not be null. | |
| /// The `child` parameter must not be null. |
| /// | ||
| /// * [InteractiveViewer.builder], whose builder is of this type. | ||
| /// * [WidgetBuilder], which is similar, but takes no Rect. | ||
| typedef TransformedWidgetBuilder = Widget Function(BuildContext context, Rect viewport); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name's a little strange. What about this is "transformed"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return value of this is transformed using a Transform widget by the InteractiveViewer. Maybe it could be InteractiveViewerWidgetBuilder if that's better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Giving this a more specific name sounds good.
|
|
||
| // Convert an axis aligned Quad to a Rect. | ||
| // | ||
| // All Rects must axis aligned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which "rects" is this referring to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of them. I'll change it to this:
"All instances of Rect are axis aligned by definition."
Meanwhile, Quads are not necessarily axis aligned.
|
|
||
| /// Returns true iff the given Quad is axis aligned. | ||
| @visibleForTesting | ||
| static bool isAxisAligned(Quad quad) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is only called from asserts, should we name this debugIsAxisAligned and be a no-op in release builds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. The asserts are automatically removed in release mode, right? I'm assuming you don't want me to check for kReleaseMode and return true and just want me to change the name.
0f8268d to
036171f
Compare
|
@goderbauer FYI I changed the builder to take a Quad instead of a Rect so that it can support rotated viewports in the future. |
This PR adds a
.builderconstructor to InteractiveViewer, similar to ListView.builder.Closes #58603
Potential fix for #49109
Why not just use a Builder as the child of InteractiveViewer?
The main value added by the builder constructor approach is the pre-calculated
viewportRect parameter. It's possible but fairly difficult to calculate this without access to the private state and methods of the InteractiveViewer instance.Example
This example shows building a Table whose cell content only renders for visible cells. In the screenshot, off-screen cells are empty until they are panned onto the screen.
Code