KRenderingGraphicsContextQuartz must DIE It pains me every time I read this code: KRenderingDeviceContext* context = device->currentContext(); bool shouldPopContext = false; if (context) paintInfo.p->save(); else { // Need to set up KCanvas rendering if it hasn't already been done. context = paintInfo.p->createRenderingDeviceContext(); device->pushContext(context); shouldPopContext = true; } . . . // restore drawing state if (!shouldPopContext) paintInfo.p->restore(); else { device->popContext(); delete context; } This is totally unnecessary. GraphicsContext should be able to do all of this for us. GraphicsContext needs to be extended and KRenderingDeviceContextQuartz (and the associated KRenderingDevice containing a stack of these buggers) must die! We can't ship SVG with this monstrosity.
I believe our current KRenderingDeviceContext usage is actually causing a host of bugs, including our printing problems.
Created attachment 10818 [details] A half finished patch to remove KRenderingDevice* I'm tired. going to bed. Eventually I'll find time to get back to this, or someone else will.
Created attachment 10853 [details] updated (but still broken) patch
Things missing from this patch include: 1. Need a KCanvasCreator to make all of these resources (since we still want to support certain platforms not implementing all of the resources). Otherwise we'd just new them where they're needed. 2. Need to augment GraphicsContext to have addPath and clearPath (or beginPath) functions. 3. Need to fix a few "context managment" places, namely: applyFilter, prepareFilter, as well as the context swapping done in Gradient text stroke drawing. I think that's it.
Created attachment 11043 [details] another completely independant (and unfinished) attempt at this
Just in case anyone is looking at this bug, I'm currently working on a new version of this patch, which will remove whole kcanvas/ directory & KRenderingDevice* classes. Attaching patch in a few hours... Niko
Created attachment 11681 [details] Initial complete patch for Cg & Qt It may still have some flaky places, but it passes layout & pixel tests w/o new regressions and even fix some (see ChangeLog)
Comment on attachment 11681 [details] Initial complete patch for Cg & Qt Looks fine. CgSupport should really be CGSupport There are a couple old spacing issues, on lines which were touched (but the spacing not corrected). I really really really really think that Image::createGraphicsContext() needs to be added ASAP. That's the *correct* way to do masking and patterns etc. The current solution is a bad hack. I would encourage you to add Image::createGraphicsContext either before this patch lands or right after. This is a huge change for someone so out-of-touch with the project to approve alone. I've reviewed the code for sanity, but I think you should have an Apple reviewer rubber stamp that this large of a change is OK to land given whatever the current project state is. (You and I had some additional discussion over IRC, and you said you had an updated patch. I'm going to r+ this one, but you should land the updated patch.)
Created attachment 11687 [details] Updated final patch Incorporated Erics comments.
Comment on attachment 11687 [details] Updated final patch I skimmed through the updated patch. WildFox agreed that he would work on the Image::createGraphicsContext() stuff next (or very soon). Also, waiting to land until someone on the Safari team and approve that landing a large patch like this is OK right now.
Landed in r17947.