[Webkit-unassigned] [Bug 77148] [Qt][Texmap] Refactor TextureMapper API to use ImageBuffers when possible.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=77148





--- Comment #28 from Noam Rosenthal <noam.rosenthal at nokia.com>  2012-02-02 10:45:22 PST ---
(In reply to comment #27)
> (From update of attachment 124715 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124715&action=review
> 
> Just a couple questions before I finish the review. Looks nice! I'm glad that we can share the ImageBuffer code with you.
> 
> > Source/WebCore/platform/graphics/GraphicsContext.h:413
> > +#if ENABLE(3D_RENDERING)
> 
> You might want to have this be ENABLE(3D_RENDERING) && USE(TEXTURE_MAPPER). Then you could move the empty implementation out of Source/WebCore/platform/graphics/GraphicsContext.cpp.
> 
> > Source/WebCore/platform/graphics/texmap/TextureMapper.cpp:193
> > +#if ENABLE(3D_RENDERING)
> > +    context->concat3DTransform(matrix);
> > +#else
> > +    context->concatCTM(matrix);
> > +#endif
> 
> This is repeated a bit. Did you consider moving the #ifdefs into concat3DTransform and the other new methods on GraphicsContext.
> 
> > Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:189
> > +    {
> > +        context->setImageInterpolationQuality(textureMapper->imageInterpolationQuality());
> > +        context->setTextDrawingMode(textureMapper->textDrawingMode());
> > +        context->translate(-dirtyRect.x(), -dirtyRect.y());
> > +        layer->paintGraphicsLayerContents(*context, dirtyRect);
> > +        if (m_currentContent.contentType == DirectImageContentType)
> > +            context->drawImage(m_currentContent.image.get(), ColorSpaceDeviceRGB, m_state.contentsRect);
> > +    }
> 
> What is the purpose of this new inner scope?
>
Not needed, it was there before because we a GraphicsContext on the stack. I'll remove that.

> > Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:191
> > +    // This suld be a reference copy only.
> 
> suld -> should
> 
> > Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:192
> > +    RefPtr<Image> image = imageBuffer->copyImage();
> 
> I believe this is a deep copy on Cairo, at least. You may need to pass a copy behavior here. Looks like Cairo has not implemented the shallow copy behavior.

Ah, yes. I need to specify DontCopyBackingStore, which is not really implemented in Qt/GTK. I don't think it's in the scope of this patch though, maybe I should add it as a FIXME?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.