[IMP] renderer: add animations on cell change #6448
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
hokolomopo
force-pushed
the
master-cell-animation-adrm
branch
8 times, most recently
from
2dd49ef to
9b7ce81
Compare
hokolomopo
commented
May 22, 2025
| icons: { left?: GridIcon; right?: GridIcon; center?: GridIcon }; | ||
| overlayColor?: Color; | ||
| icons: { left?: GridIconWithOpacity; right?: GridIconWithOpacity; center?: GridIconWithOpacity }; | ||
| textOpacity?: number; |
There was a problem hiding this comment.
could also add textOpacity to property style of Box
| for (const { zone, rect } of this.getters.getAllActiveViewportsZonesAndRect()) { | ||
| const { ctx } = renderingContext; | ||
| ctx.save(); | ||
| ctx.beginPath(); | ||
| ctx.rect(rect.x, rect.y, rect.width, rect.height); | ||
| ctx.clip(); | ||
| const boxes = this.getGridBoxes(zone); | ||
| const boxesWithoutAnimations = this.getGridBoxes(zone); |
There was a problem hiding this comment.
could optimize the code a bit there; computing a new Box is useless if we're in an animation frame that isn't caused by a model update. We could re-use the boxes of last render. A bit annoying (because of multiple viewports), but entirely possible.
We'd save 2-3ms at each animation frame.
hokolomopo
force-pushed
the
master-cell-animation-adrm
branch
2 times, most recently
from
066c5b0 to
2216143
Compare
hokolomopo
added a commit
that referenced
this pull request
May 27, 2025
This commit replaces the HTML-based rendering of grid icons with a canvas-based approach. This have the advantages of: - improved performance, especially when there are a lot of icons (no DOM + OWL manipulation overhead) - no parallax effect on mobile devices - is compatible with cell animations of #6448 Task: 4674384
hokolomopo
added a commit
that referenced
this pull request
Jun 2, 2025
This commit replaces the HTML-based rendering of grid icons with a canvas-based approach. This have the advantages of: - improved performance, especially when there are a lot of icons (no DOM + OWL manipulation overhead) - no parallax effect on mobile devices - is compatible with cell animations of #6448 Task: 4674384
robodoo
pushed a commit
that referenced
this pull request
Jun 2, 2025
This commit replaces the HTML-based rendering of grid icons with a canvas-based approach. This have the advantages of: - improved performance, especially when there are a lot of icons (no DOM + OWL manipulation overhead) - no parallax effect on mobile devices - is compatible with cell animations of #6448 Task: 4674384 Part-of: #6111 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
hokolomopo
force-pushed
the
master-cell-animation-adrm
branch
from
2216143 to
4a2f2f9
Compare
In the rendering process, the `verticalAlign` property is duplicated: inside a `Box` there is both a `verticalAlign` and a `style.verticalAlign` property. This commit removes the `verticalAlign` property from the `Box`. Task: 4805149
The function `computeTextWidth` uses a cache to store the width of a text, but we would make canvas operations `ctx.save()/restore()` even if the width is already in the cache. For an animation on 350 cells, we went from spending 25ms in `computeTextWidth` to 4ms. Task: 4805149
This commit adds animations in the grid's canvas when a cell is changed. The animations are defined in the `cell_animation_registry.ts` and includes: - fade in the cell when it gets a new content. - fate out the cell when its content is removed. - text sliding animation when the text is changed. - color gradient animation when the cell colors is changed - and more The feature is designed to not impact the current grid drawing logic. We still have a first step yo create `Box` from the model content, and a second step to draw the `Box` in the canvas. The animations are a step in between, where we change the `Box` depending on the animation progress, and possibly add some additional `Box` to be drawn. Task: 4805149
hokolomopo
force-pushed
the
master-cell-animation-adrm
branch
from
4a2f2f9 to
62c9513
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This commit adds animations in the grid's canvas when a cell is changed.
The animations are defined in the
cell_animation_registry.tsandincludes:
The feature is designed to not impact the current grid drawing logic.
We still have a first step yo create
Boxfrom the model content,and a second step to draw the
Boxin the canvas. The animationsare a step in between, where we change the
Boxdepending on theanimation progress, and possibly add some additional
Boxto be drawn.Task: 4805149
review checklist