Editable elements are a repaint boundary #396
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #396 +/- ##
==========================================
- Coverage 87.94% 87.93% -0.01%
==========================================
Files 61 61
Lines 10311 10312 +1
==========================================
Hits 9068 9068
- Misses 1243 1244 +1 ☔ View full report in Codecov by Sentry. |
amantoux
force-pushed
the
fix/cursor_blink_gpu_intensive
branch
from
5c56b1c
to
9798d06
Compare
amantoux
changed the title
amantoux
changed the title
|
@Amir-P can you please review? |
amantoux
changed the title
There was a problem hiding this comment.
Thanks! LGTM overall. I just had a question and left you in the comments. PTAL.
Also here are the performance results against master (from the tool added in #384):
Analyzing tests for scrolling
Average Frame Build Time: Target: 57.08 Reference: 58.51
90th Percentile Frame Build Time: Target: 65.83 Reference: 62.51
90th Percentile Frame Build Time: Target: 68.48 Reference: 66.92
Average GPU Frame Time: Target: 0.00 Reference: 0.00
Performance tests found no regression
Analyzing tests for editing
Average Frame Build Time: Target: 430.64 Reference: 712.07
90th Percentile Frame Build Time: Target: 434.68 Reference: 715.65
90th Percentile Frame Build Time: Target: 435.07 Reference: 716.17
Average GPU Frame Time: Target: 0.00 Reference: 0.00
Performance tests found no regression
| @@ -861,6 +861,7 @@ class RenderEditor extends RenderEditableContainerBox | |||
| _floatingCursorRect = null; | |||
| _cursorController.setFloatingCursorTextPosition(null); | |||
| } | |||
| markNeedsPaint(); | |||
There was a problem hiding this comment.
I've experiment with and without this line and saw no changes. Can you explain why it was added? And maybe add a test for that so we don't forget in the future?
There was a problem hiding this comment.
The floating cursor stay until the next tap or edit action
In the video, notice the floating cursor that remains after releasing the floating cursor
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-08-16.at.18.23.14.mp4
There was a problem hiding this comment.
Then I guess no need for a test. It would be really hard to test it I guess.
Closes #394