Skip to content
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

CanvasVectorLayerRenderer sets canvas width to zero on every render #12791

Closed
strax opened this issue Sep 21, 2021 · 5 comments · Fixed by #12792
Closed

CanvasVectorLayerRenderer sets canvas width to zero on every render #12791

strax opened this issue Sep 21, 2021 · 5 comments · Fixed by #12792
Labels

Comments

@strax
Copy link

strax commented Sep 21, 2021

Describe the bug
When an empty VectorLayer is followed by other layers, the combined canvas is resized to canvas.width=0 on every render after which the width is restored. This seems to negatively affect performance compared to clearing the canvas with CanvasRenderingContext2D.clearRect, which is used in other renderers (CanvasTileLayerRenderer, CanvasImageLayerRenderer).

To Reproduce
Reproduction example at https://t1d1e.csb.app/, sources https://codesandbox.io/s/great-sky-t1d1e.

Expected behavior
CanvasVectorLayerRenderer should probably use clearRect to clear the canvas instead of clearing it by setting the canvas ' width to zero.

@strax strax added the bug label Sep 21, 2021
@ahocevar
Copy link
Member

Good find! The idea was to zero the size and not touch it again until the layer is made visible again. If it really gets resized twice on every render, we need to fix something.

@ahocevar
Copy link
Member

I just tried this. Are you talking about

if (!this.containerReused && canvas.width > 0) {
canvas.width = 0;
}
? With your example, I never get into that if block, because an existing container is reused. Or do you mean something else?

@strax
Copy link
Author

strax commented Sep 21, 2021

Yeah, that's the section I'm talking about.

With your example, I never get into that if block, because an existing container is reused

That's odd, the phenomenon is definitely happening for me (Chrome 93). In the repro page, you can attach a breakpoint on "attribute modifications" on the .ol-layer > canvas DOM element (not the .base canvas) and then any map interaction should pause on that line. In that state, it can be observed that the width of the canvas is 0 and continued execution will reset it back to the width of the map viewport.

The first breakpoint where the width gets set to zero:
Screenshot 2021-09-21 at 12 58 39

Continuing execution from there, prepareFrame sets the width back immediately:
Screenshot 2021-09-21 at 12 59 07

@ahocevar
Copy link
Member

Ok, I see what you mean. Interestingly, I can only reproduce this in your codesandbox. When I run the exact same code locally, it does not happen. Can you reproduce that locally?

Anyway, I created #12792, which should fix this issue. Can you please try with that branch and see if you notice any irregularities there?

@strax
Copy link
Author

strax commented Sep 22, 2021

Yep, the issue could be reproduced locally too.

I just tested #12792 and seems like it fixes the issue! I didn't notice any visible regressions either when there are other layers than the VectorLayer rendering to the same canvas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants