fix(web): optimize and fix present_with_damage #150
Merged
Conversation
This file contains 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
CBenoit
commented
Sep 8, 2023
Merged
|
I addressed the clippy lint. |
CBenoit
commented
Sep 8, 2023
daxpedda
force-pushed
the
web-fix-put-image
branch
from
b600089
to
1ccd3eb
Compare
notgull
reviewed
Sep 9, 2023
daxpedda
reviewed
Sep 9, 2023
daxpedda
force-pushed
the
web-fix-put-image
branch
2 times, most recently
from
52abefe
to
d4a6f5e
Compare
This patch fixes the present_with_damage implementation for the web backend. `CanvasRenderingContext2D`’s `putImageDate` was used incorrectly. As per Mozilla documentation: > imageData > > An ImageData object containing the array of pixel values. > > dx > > Horizontal position (x coordinate) at which to place the image data > in the destination canvas. > > dy > > Vertical position (y coordinate) at which to place the image data > in the destination canvas. > > dirtyX > > Horizontal position (x coordinate) of the top-left corner from which > the image data will be extracted. Defaults to 0. > > dirtyY > > Vertical position (y coordinate) of the top-left corner from which > the image data will be extracted. Defaults to 0. As such, `dx` is different from `dirtyX` and `dy` is different from `dirtyY`. When the actual `ImageData` is as big as the canvas itself, `dx` and `dy` should always be `0.0` and `0.0`, and only `dirtyX` and `dirtyY` are actually representing the top-left coordinates for the dirty region. The dirty region is relative to the image data. This patch also optimizes the BGRX to RGBA conversion by only creating a bitmap for the smallest region enclosing all the damaged regions. This does not affect the behavior of `present`.
daxpedda
force-pushed
the
web-fix-put-image
branch
from
4aa4563
to
94f2984
Compare
|
Just waiting for somebody to take a look at #150 (comment) before I merge this. @notgull, would be appreciated. |
|
Thank you for the prompt review! |
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.
This patch fixes the
present_with_damageimplementation for the web backend.CanvasRenderingContext2D’sputImageDatawas used incorrectly.As per Mozilla documentation:
As such,
dxis different fromdirtyXanddyis different fromdirtyY.When the actual
ImageDatais as big as the canvas itself,dxanddyshould always be0.0and0.0, and onlydirtyXanddirtyYare actually representing the top-left coordinates for the dirty region. The dirty region is relative to the image data.This patch also optimizes the BGRX to RGBA conversion by only creating a bitmap for the smallest region enclosing all the damaged regions. This does not affect the behavior of
present.