Skip to content

Commit

Permalink
Fix ImageBufferQt scaling wrong for HiDPI environments
Browse files Browse the repository at this point in the history
As per #461 - the scale was not applied to the source,
resulting in the filter attempting to copy from unallocated memory.

This adds similar implementation of scaling to the one from the
ImageBufferCG changes in https://trac.webkit.org/changeset/168577/webkit
("-webkit-filter prevents rendering at retina scale")

Also fixes putByteArray creating too small image, and adds proper
scale to images.

Change-Id: I39a38097ce017e9688db789478c292f37038abe3
  • Loading branch information
jkozera authored and annulen committed May 17, 2017
1 parent d6d0603 commit 70d9f8a
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 21 deletions.
20 changes: 14 additions & 6 deletions Source/WebCore/platform/graphics/qt/ImageBufferDataQt.cpp
Expand Up @@ -351,7 +351,7 @@ GraphicsSurfaceToken ImageBufferDataPrivateAccelerated::graphicsSurfaceToken() c
// ---------------------- ImageBufferDataPrivateUnaccelerated

struct ImageBufferDataPrivateUnaccelerated final : public ImageBufferDataPrivate {
ImageBufferDataPrivateUnaccelerated(const IntSize&);
ImageBufferDataPrivateUnaccelerated(const IntSize&, float scale);
QPaintDevice* paintDevice() final { return m_pixmap.isNull() ? 0 : &m_pixmap; }
QImage toQImage() const final;
RefPtr<Image> image() const final;
Expand All @@ -371,11 +371,13 @@ struct ImageBufferDataPrivateUnaccelerated final : public ImageBufferDataPrivate
RefPtr<Image> m_image;
};

ImageBufferDataPrivateUnaccelerated::ImageBufferDataPrivateUnaccelerated(const IntSize& size)
ImageBufferDataPrivateUnaccelerated::ImageBufferDataPrivateUnaccelerated(const IntSize& size, float scale)
: m_pixmap(size)
, m_image(StillImage::createForRendering(&m_pixmap))
{
m_pixmap.fill(QColor(Qt::transparent));
// needs proper pixel ratio for drawing on HiDPI devices:
m_pixmap.setDevicePixelRatio(scale);
}

QImage ImageBufferDataPrivateUnaccelerated::toQImage() const
Expand Down Expand Up @@ -474,11 +476,14 @@ void ImageBufferDataPrivateUnaccelerated::platformTransformColorSpace(const Vect

// ---------------------- ImageBufferData

ImageBufferData::ImageBufferData(const IntSize& size)
ImageBufferData::ImageBufferData(const FloatSize& size, float resolutionScale)
{
m_painter = new QPainter;

m_impl = new ImageBufferDataPrivateUnaccelerated(size);
// we need to copy it, to scale without converting to int first, to retain precision:
FloatSize scaledSize(size);
scaledSize.scale(resolutionScale);
m_impl = new ImageBufferDataPrivateUnaccelerated(IntSize(scaledSize), resolutionScale);

if (!m_impl->paintDevice())
return;
Expand All @@ -489,11 +494,14 @@ ImageBufferData::ImageBufferData(const IntSize& size)
}

#if ENABLE(ACCELERATED_2D_CANVAS)
ImageBufferData::ImageBufferData(const IntSize& size, QOpenGLContext* compatibleContext)
ImageBufferData::ImageBufferData(const FloatSize& size, float resolutionScale, QOpenGLContext* compatibleContext)
{
m_painter = new QPainter;

m_impl = new ImageBufferDataPrivateAccelerated(size, compatibleContext);
// we need to copy it, to scale without converting to int first, to retain precision:
FloatSize scaledSize(size);
scaledSize.scale(resolutionScale);
m_impl = new ImageBufferDataPrivateAccelerated(IntSize(scaledSize), compatibleContext);

if (!m_impl->paintDevice())
return;
Expand Down
4 changes: 2 additions & 2 deletions Source/WebCore/platform/graphics/qt/ImageBufferDataQt.h
Expand Up @@ -65,9 +65,9 @@ struct ImageBufferDataPrivate {

class ImageBufferData {
public:
ImageBufferData(const IntSize&);
ImageBufferData(const FloatSize&, float resolutionScale);
#if ENABLE(ACCELERATED_2D_CANVAS)
ImageBufferData(const IntSize&, QOpenGLContext*);
ImageBufferData(const FloatSize&, float resolutionScale, QOpenGLContext*);
#endif
~ImageBufferData();
QPainter* m_painter;
Expand Down
49 changes: 36 additions & 13 deletions Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp
Expand Up @@ -51,28 +51,33 @@
namespace WebCore {

#if ENABLE(ACCELERATED_2D_CANVAS)
ImageBuffer::ImageBuffer(const IntSize& size, float /* resolutionScale */, ColorSpace, QOpenGLContext* compatibleContext, bool& success)
: m_data(size, compatibleContext)
ImageBuffer::ImageBuffer(const IntSize& size, float resolutionScale, ColorSpace, QOpenGLContext* compatibleContext, bool& success)
: m_data(size, resolutionScale, compatibleContext)
, m_size(size)
, m_logicalSize(size)
, m_resolutionScale(resolutionScale)
{
success = m_data.m_painter && m_data.m_painter->isActive();
if (!success)
return;

m_size.scale(resolutionScale);

m_data.m_context = std::make_unique<GraphicsContext>(m_data.m_painter);
}
#endif

ImageBuffer::ImageBuffer(const FloatSize& size, float /* resolutionScale */, ColorSpace, RenderingMode /*renderingMode*/, bool& success)
: m_data(IntSize(size))
, m_size(size)
ImageBuffer::ImageBuffer(const FloatSize& size, float resolutionScale, ColorSpace, RenderingMode /*renderingMode*/, bool& success)
: m_data(size, resolutionScale)
, m_logicalSize(size)
, m_resolutionScale(resolutionScale)
{
success = m_data.m_painter && m_data.m_painter->isActive();
if (!success)
return;

m_size = IntSize(size * resolutionScale);

m_data.m_context = std::make_unique<GraphicsContext>(m_data.m_painter);
}

Expand Down Expand Up @@ -139,8 +144,14 @@ void ImageBuffer::platformTransformColorSpace(const Vector<int>& lookUpTable)
}

template <Multiply multiplied>
PassRefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const ImageBufferData& imageData, const IntSize& size)
PassRefPtr<Uint8ClampedArray> getImageData(const IntRect& unscaledRect, float scale, const ImageBufferData& imageData, const IntSize& size,
ImageBuffer::CoordinateSystem coordinateSystem)
{
IntRect rect(unscaledRect);

if (coordinateSystem == ImageBuffer::LogicalCoordinateSystem)
rect.scale(scale);

float area = 4.0f * rect.width() * rect.height();
if (area > static_cast<float>(std::numeric_limits<int>::max()))
return 0;
Expand All @@ -155,24 +166,27 @@ PassRefPtr<Uint8ClampedArray> getImageData(const IntRect& rect, const ImageBuffe
// Let drawImage deal with the conversion.
// FIXME: This is inefficient for accelerated ImageBuffers when only part of the imageData is read.
QPainter painter(&image);
// this painter needs the same scaling as the whole buffer, to copy HiDPI images properly:
if (coordinateSystem == ImageBuffer::LogicalCoordinateSystem)
painter.setWorldTransform(QTransform::fromScale(scale, scale));
painter.setCompositionMode(QPainter::CompositionMode_Source);
painter.drawImage(QPoint(0, 0), imageData.m_impl->toQImage(), rect);
painter.end();

return result.release();
}

PassRefPtr<Uint8ClampedArray> ImageBuffer::getUnmultipliedImageData(const IntRect& rect, CoordinateSystem) const
PassRefPtr<Uint8ClampedArray> ImageBuffer::getUnmultipliedImageData(const IntRect& rect, CoordinateSystem coordinateSystem) const
{
return getImageData<Unmultiplied>(rect, m_data, m_size);
return getImageData<Unmultiplied>(rect, m_resolutionScale, m_data, m_size, coordinateSystem);
}

PassRefPtr<Uint8ClampedArray> ImageBuffer::getPremultipliedImageData(const IntRect& rect, CoordinateSystem) const
PassRefPtr<Uint8ClampedArray> ImageBuffer::getPremultipliedImageData(const IntRect& rect, CoordinateSystem coordinateSystem) const
{
return getImageData<Premultiplied>(rect, m_data, m_size);
return getImageData<Premultiplied>(rect, m_resolutionScale, m_data, m_size, coordinateSystem);
}

void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, CoordinateSystem)
void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, const IntSize& sourceSize, const IntRect& sourceRect, const IntPoint& destPoint, CoordinateSystem coordinateSystem)
{
ASSERT(sourceRect.width() > 0);
ASSERT(sourceRect.height() > 0);
Expand All @@ -189,12 +203,21 @@ void ImageBuffer::putByteArray(Multiply multiplied, Uint8ClampedArray* source, c
m_data.m_painter->setClipping(false);
}

// source rect & size need scaling from the device coords to image coords
IntSize scaledSourceSize(sourceSize);
IntRect scaledSourceRect(sourceRect);
if (coordinateSystem == LogicalCoordinateSystem) {
scaledSourceSize.scale(m_resolutionScale);
scaledSourceRect.scale(m_resolutionScale);
}

// Let drawImage deal with the conversion.
QImage::Format format = (multiplied == Unmultiplied) ? QImage::Format_RGBA8888 : QImage::Format_RGBA8888_Premultiplied;
QImage image(source->data(), sourceSize.width(), sourceSize.height(), format);
QImage image(source->data(), scaledSourceSize.width(), scaledSourceSize.height(), format);
image.setDevicePixelRatio(m_resolutionScale);

m_data.m_painter->setCompositionMode(QPainter::CompositionMode_Source);
m_data.m_painter->drawImage(destPoint + sourceRect.location(), image, sourceRect);
m_data.m_painter->drawImage(destPoint + sourceRect.location(), image, scaledSourceRect);

if (!isPainting)
m_data.m_painter->end();
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/platform/graphics/qt/StillImageQt.cpp
Expand Up @@ -83,6 +83,9 @@ void StillImage::draw(GraphicsContext& ctxt, const FloatRect& dst,
FloatRect normalizedSrc = src.normalized();
FloatRect normalizedDst = dst.normalized();

// source rect needs scaling from the device coords to image coords
normalizedSrc.scale(m_pixmap->devicePixelRatio());

CompositeOperator previousOperator = ctxt.compositeOperation();
BlendMode previousBlendMode = ctxt.blendModeOperation();
ctxt.setCompositeOperation(op, blendMode);
Expand Down

0 comments on commit 70d9f8a

Please sign in to comment.