1289975 - Do not reset layer when setting the same dimension to canvas

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Ethan Lin[:ethlin]]

We always reset layer when doing setDimension. I think we can reuse the layer and buffer if the dimension is the same. Then the layer doesn't need to realloate buffer.

Comment 1

[Ethan Lin[:ethlin]]

:nical, do you think we should do it?

Comment 2

[Nicolas Silva [:nical]]

Per spec, touching the canvas size (even without changing it) resets the canvas drawing state. If we want to reuse the buffer, it's fine but we need to be very careful about not keeping around any additional drawing state like transforms, clips or whatnot.

I don't feel very strongly for or against keeping the canvas buffer in that case as long as it doesn't complicate the code too much, although I would rather encourage web authors to not use SetDimension as a way to clear the canvas since it is going to be slower in any case.

Do you have an idea of how often web authors touch the dimensions of a canvas without changing the actual values?

Comment 3

[Ethan Lin[:ethlin]]

(In reply to Nicolas Silva [:nical] from comment #2)
> Per spec, touching the canvas size (even without changing it) resets the
> canvas drawing state. If we want to reuse the buffer, it's fine but we need
> to be very careful about not keeping around any additional drawing state
> like transforms, clips or whatnot.
> 
> I don't feel very strongly for or against keeping the canvas buffer in that
> case as long as it doesn't complicate the code too much, although I would
> rather encourage web authors to not use SetDimension as a way to clear the
> canvas since it is going to be slower in any case.
> 
> Do you have an idea of how often web authors touch the dimensions of a
> canvas without changing the actual values?

I don't know how often the web authors set the same dimension. But I tried this case on chrome and safari and I believe they reused the buffer. From my test, if a page keeps setting the same dimension to a large canvas, firefox will use a lot of CPU resource and will have memory fragment problem in the end, then crash.
So yes, if the code doesn't complicate, I think it's good to have the reusing buffer mechanism.

Comment 4

[Ethan Lin[:ethlin]]

Attachment: reuse buffer.

Keep the buffer provider while the dimension is the same.

Comment 5

[Ethan Lin[:ethlin]]

Attachment: testcase

This html keeps setting the same dimension for each frame.

Comment 6

[Ethan Lin[:ethlin]]

Attachment: reuse buffer

In this patch, I check the dimension before ClearTarget. If the dimension is the same, then keep the BufferProvider and clear the buffer data.
I tried the testcase attachment 8777213 [details] With BufferProviderShared, the FPS increased to 24 from 18 on windows.

Comment 7

[Ethan Lin[:ethlin]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=679d0a829af2&selectedJob=25086301

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3758ab7ee78
Reuse canvas buffer when setting the same dimension. r=nical

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e3758ab7ee78