296907 - fill/stroke shouldn't destroy path

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[ArronM (:paper)]

I should be able to build a nice path, then stroke and fill it.  Currently, if I
made a very complex path, I'd have to stroke it, then rebuild the same path,
then fill it.  That's a waste of CPU time.

The spec currently says

"Paths, when filled or stroked, are painted without affecting the current path,"

as well as (in bold italic green, I might add)

"The current path is persistent, and can only be reset using the beginPath()
method."

Comment 1

[Hixie (not reading bugmail)]

Now this bug is definitely valid IMHO.

Comment 2

[rossi]

well i would attach a patch, but after a system crash i have to set everything up again and i don't have much time so i don't know when i'll be ready. until then i'd love to see someone change

nsCanvasRenderingContext2D.cpp
line 1070
from
cairo_fill(mCairo);
to
cairo_fill_preserve(mCairo);
and line 1078
from
cairo_stroke(mCairo);
to cairo_stroke_preserve(mCairo);

thanks.

this really slows down my 3d engine... =)

Comment 3

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

Attachment: canvas-path-preserve.patch

Argh.. we couldn't fix this on the branch until we took the late update to cairo 1.0, and I forgot that this was still outstanding at that point.

Comment 4

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

Small patch that fixes spec compatability with <canvas>.  We couldn't do this before due to cairo, but it became possible once cairo 1.0 landed on the branch (at which point I promptly forgot that this was waiting on cairo 1.0, ugh.).  There are workarounds for this (drawing the path twice if someone wants to fill and stroke), so this can also wait for 1.8.1.

Comment 5

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

-> 1.8.1

Comment 6

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

Can I beg for this for any possible RC before 1.5? :]

Comment 7

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

Checked in on trunk; low risk patch for 1.8.0.1 and fixes canvas spec-compliance.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 201872 [details] [diff] [review]
canvas-path-preserve.patch

a=dveditz for drivers