293267 - When called via onclick, canvas rendering is far slower than called via onload

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Jens Bannmann]

When drawing into a canvas, the speed heavily differs depending on the way the
function is invoked. onload rendering is okay, onclick/href="javascript:..." is
really slow. Is this a bug or the intended behaviour?

Comment 1

[Jens Bannmann]

Attachment: canvas demo started via onclick

Comment 2

[Jens Bannmann]

Attachment: canvas demo started via onload

Comment 3

[Sjoerd Visscher]

What may be causing this is that after initial loading the canvas seems to be
rendered to screen after every drawing call. But that should not happen during
javascript execution. Only when the script is finished should the canvas repaint
to screen again.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: massive canvas speedup patch

Whoops, this was a pretty big thinko in the implementation.  This patch speeds
up performance drastically for interactive renderings.	It's still going to be
slightly /slow/ until the rendering stuff gets a little deeper into our moz
rendering core, but you won't get to watch each individual circle paint.

As a side effect, this also fixes the bug 293544
holding-pointer-after-Release() issue.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

-    NS_IF_ADDREF(*canvas = mDOMCanvasElement);
-    return NS_OK;
+    return CallQueryInterface(mCanvasElement, canvas);

You probably need to nullcheck mCanvasElement first since CallQI isn't nullsafe.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 183361 [details] [diff] [review]
massive canvas speedup patch

>Index: content/canvas/src/nsCanvasRenderingContext2D.cpp
> nsCanvasRenderingContext2D::GetCanvas(nsIDOMHTMLCanvasElement **canvas)
> {
>-    NS_IF_ADDREF(*canvas = mDOMCanvasElement);
>-    return NS_OK;
>+    return CallQueryInterface(mCanvasElement, canvas);

That will crash if mCanvasElement is null.  You need to explicitly set *canvas
to null in that case, and not CallQI.

>Index: content/html/content/src/nsHTMLCanvasElement.cpp
>+  NS_IMETHOD UpdateImageFrame ();

Nix space before '('.

>+nsHTMLCanvasElement::UpdateImageFrame ()

And here.

r+sr=bzbarsky with those fixed.

Comment 7

[Mike Shaver (:shaver emeritus)]

Comment on attachment 183361 [details] [diff] [review]
massive canvas speedup patch

a=shaver

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in with CallQI and whitespace issues fixed.  One of these days I'll
remember that CallQI doesn't handle a null param...

Comment 9

[Peter van der Woude [:Peter6]]

confirmed, the difference is really unreal
testcase 1 went from 15 secs down to instant