[Webkit-unassigned] [Bug 78305] [Qt][Texmap] Refactor backing-store code in TextureMapper

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2012-02-10 00:59:41 PST ---
(From update of attachment 126428)
View in context: https://bugs.webkit.org/attachment.cgi?id=126428&action=review

> Source/WebCore/platform/graphics/opengl/TextureMapperGL.h:49
> +    virtual int maxTextureDimension() { return 2000; }

Would it make sense to make it possible to use different dimentions horizontally, vertically?

> Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:304
> +    m_compositedImage = TextureMapperCompositedImage::get(m_image.get(), textureMapper);
> +    m_contentsLayer = m_media ? m_media : m_compositedImage.get();

Would it make sense to check for m_media first?

> Source/WebCore/platform/graphics/texmap/TextureMapper.h:109
> +    virtual int maxTextureDimension() { return 10000; }

Why such a crazy big default? if you want to avoid tiling why not use MAX_INT or so.

> Source/WebCore/platform/graphics/texmap/TextureMapperBackingStore.cpp:31
> +void TextureMapperTile::updateContents(TextureMapper* textureMapper, Image* image, const IntRect& dirtyRect)
> +{
> +    IntRect targetRect = enclosingIntRect(m_rect);

Please notice that webkit just switched to floats for layouts. Maybe we soon need to change to using LayoutRects instead

> Source/WebCore/platform/graphics/texmap/TextureMapperBackingStore.cpp:44
> +        m_texture->reset(targetRect.size(), false);

specify what the false means

> Source/WebCore/platform/graphics/texmap/TextureMapperBackingStore.cpp:59
> +void TextureMapperBackingStore::computeTilesIfNeeded(const FloatSize& size, int tileSize)

It seems more like a createTilesIfNeeded()

> Source/WebCore/platform/graphics/texmap/TextureMapperBackingStore.cpp:80
> +        bool found = false;

bool existsAlready = false?

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:82
> +PassRefPtr<TextureMapperBackingStore> TextureMapperNode::getBackingStore()

why not just backingStore() ?

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:99
> +    FloatRect dirtyRect = m_state.needsDisplay ? layerRect() : m_state.needsDisplayRect;

is this correct?

> Source/WebKit2/UIProcess/qt/LayerBackingStore.cpp:49
> +#if PLATFORM(QT)
> +    QImage qtImage = m_backBuffer->createQImage();
> +    pixels = qtImage.constBits();
> +#endif

You intent this file to move out of /qt/ ?

> Source/WebKit2/UIProcess/qt/LayerBackingStore.cpp:50
> +    texture->updateContents(pixels, IntRect(IntPoint(), m_backBuffer->size()));

why not use ::zero() here

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