Bug 7444 – move QPainter to platform directory and name it GraphicsContext

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

Here's the next step in de-KWQ'ing.

[Darin Adler]

Created attachment 6699 [details]
patch

[Darin Adler]

Created attachment 6710 [details]
patch, updated for changes to TOT

[Darin Adler]

Created attachment 6742 [details]
patch, improved a bit

[Eric Seidel (no email)]

Comment on attachment 6742 [details]
patch, improved a bit

I think this is backwards:

-    return new KRenderingDeviceContextQuartz(currentContext());
+    return new KRenderingDeviceContextQuartz((CGContextRef)[[NSGraphicsContext currentContext] graphicsPort]);


I'm not quite sure why this ended up in GraphicsContextCG:

+void GraphicsContext::drawImageAtPoint(Image* image, const IntPoint& p, Image::CompositeOperator compositeOperator)

Maybe I'm reading the patch backwords, but it looks like very little (at least very little cg specific stuff) ended up in GraphicsContextCG

render_canvasimage.cpp:
+            oldOperation = GraphicsContext::getCompositeOperation(GraphicsContext::currentCGContext());

This seems to beg for further simplicification, but I guess that's the next patch.

If you'd like you could also remove the DOM part of DOMString in RenderText.h.  I assume you just did a s/DOM:://

This patch has your markup.cpp changes in it as well.  Those are long since landed, iirc.

I'm not sure this assertion is still necessary:

-    ASSERT(currentCGContext() == QPainter().currentContext());
+    ASSERT(currentCGContext() == [[NSGraphicsContext currentContext] graphicsPort]);

I reviewed the whole thing.  Looks great.  It may need a plt check, but it doesn't look like any of this should affect performance.  r=me.

[Darin Adler]

(In reply to comment #4)
> (From update of attachment 6742 [details] [edit])
> I think this is backwards:
> 
> -    return new KRenderingDeviceContextQuartz(currentContext());
> +    return new KRenderingDeviceContextQuartz((CGContextRef)[[NSGraphicsContext
> currentContext] graphicsPort]);

Fixed.

> I'm not quite sure why this ended up in GraphicsContextCG:
> 
> +void GraphicsContext::drawImageAtPoint(Image* image, const IntPoint& p,
> Image::CompositeOperator compositeOperator)

I just put everything I could into the CG file.

> Maybe I'm reading the patch backwords, but it looks like very little (at least
> very little cg specific stuff) ended up in GraphicsContextCG

That's right. Right now because of the private state everything has to be in the Mac file. We should fine a way to fix that.

> render_canvasimage.cpp:
> +            oldOperation =
> GraphicsContext::getCompositeOperation(GraphicsContext::currentCGContext());
> 
> This seems to beg for further simplicification, but I guess that's the next
> patch.

Exactly.

> If you'd like you could also remove the DOM part of DOMString in RenderText.h. 
> I assume you just did a s/DOM:://

I did it.

> This patch has your markup.cpp changes in it as well.  Those are long since
> landed, iirc.

Actually those are unwanted. I won't land them.

> I'm not sure this assertion is still necessary:
> 
> -    ASSERT(currentCGContext() == QPainter().currentContext());
> +    ASSERT(currentCGContext() == [[NSGraphicsContext currentContext]
> graphicsPort]);

Looks useless to me.