Commit f5ff44ae authored by bsep's avatar bsep Committed by Commit bot

Change image document zooming logic.

The current zoom logic in image documents interacts badly with use-zoom-for-dsf,
in addition to having some weird behavior already.

This patch changes three things:
1. When loading a large image when page zoom was not 100% the image would start
   at a strange scaling factor until it was clicked. This is fixed.
2. When page zoom is at 100% a large image will always scale to fit the window
   regardless of the device scale factor. Previously with use-zoom-for-dsf on it
   would scale too large for the window.
3. When an image that fits in the window is zoomed in until it doesn't it is no
   longer clickable. Previously it would start acting like a large image, which
   was strange.

Also added a test suite for ImageDocument.

BUG=324086,644059

Review-Url: https://codereview.chromium.org/2319863006
Cr-Commit-Position: refs/heads/master@{#419867}
parent 385a310b
......@@ -61,6 +61,9 @@ crbug.com/591500 [ Win10 ] compositing/squashing/squashing-print.html [ Failure
crbug.com/598051 paint/invalidation/svg/absolute-sized-content-with-resources.xhtml [ Skip ]
crbug.com/598051 svg/zoom/page/zoom-foreignObject.svg [ Skip ]
crbug.com/644059 fast/images/image-zoom-to-25.html [ NeedsRebaseline ]
crbug.com/644059 virtual/gpu-rasterization/fast/images/image-zoom-to-25.html [ NeedsRebaseline ]
crbug.com/602110 hittesting/border-hittest-with-image-fallback.html [ Failure ]
crbug.com/606302 [ Win7 Debug ] compositing/perpendicular-layer-sorting.html [ Failure ]
......
......@@ -1101,6 +1101,7 @@ source_set("unit_tests") {
"html/HTMLTextAreaElementTest.cpp",
"html/HTMLTextFormControlElementTest.cpp",
"html/HTMLVideoElementTest.cpp",
"html/ImageDocumentTest.cpp",
"html/LinkRelAttributeTest.cpp",
"html/TimeRangesTest.cpp",
"html/canvas/CanvasAsyncBlobCreatorTest.cpp",
......
......@@ -46,6 +46,7 @@
#include "core/loader/DocumentLoader.h"
#include "core/loader/FrameLoader.h"
#include "core/loader/FrameLoaderClient.h"
#include "platform/HostWindow.h"
#include "wtf/text/StringBuilder.h"
#include <limits>
......@@ -247,32 +248,37 @@ void ImageDocument::createDocumentStructure()
float ImageDocument::scale() const
{
DCHECK_EQ(m_shrinkToFitMode, Desktop);
if (!m_imageElement || m_imageElement->document() != this)
return 1.0f;
FrameView* view = frame()->view();
if (!view)
return 1;
return 1.0f;
DCHECK(m_imageElement->cachedImage());
LayoutSize imageSize = m_imageElement->cachedImage()->imageSize(LayoutObject::shouldRespectImageOrientation(m_imageElement->layoutObject()), pageZoomFactor(this));
LayoutSize windowSize = LayoutSize(view->width(), view->height());
const float zoom = pageZoomFactor(this);
LayoutSize imageSize = m_imageElement->cachedImage()->imageSize(LayoutObject::shouldRespectImageOrientation(m_imageElement->layoutObject()), zoom);
float widthScale = windowSize.width().toFloat() / imageSize.width().toFloat();
float heightScale = windowSize.height().toFloat() / imageSize.height().toFloat();
// We want to pretend the viewport is larger when the user has zoomed the
// page in (but not when the zoom is coming from device scale).
const float manualZoom = zoom / view->getHostWindow()->windowToViewportScalar(1.f);
float widthScale = view->width() * manualZoom / imageSize.width().toFloat();
float heightScale = view->height() * manualZoom / imageSize.height().toFloat();
return min(widthScale, heightScale);
}
void ImageDocument::resizeImageToFit(ScaleType type)
void ImageDocument::resizeImageToFit()
{
if (!m_imageElement || m_imageElement->document() != this || (pageZoomFactor(this) > 1 && type == ScaleOnlyUnzoomedDocument))
DCHECK_EQ(m_shrinkToFitMode, Desktop);
if (!m_imageElement || m_imageElement->document() != this)
return;
DCHECK(m_imageElement->cachedImage());
LayoutSize imageSize = m_imageElement->cachedImage()->imageSize(LayoutObject::shouldRespectImageOrientation(m_imageElement->layoutObject()), pageZoomFactor(this));
LayoutSize imageSize = m_imageElement->cachedImage()->imageSize(LayoutObject::shouldRespectImageOrientation(m_imageElement->layoutObject()), 1.f);
float scale = this->scale();
const float scale = this->scale();
m_imageElement->setWidth(static_cast<int>(imageSize.width() * scale));
m_imageElement->setHeight(static_cast<int>(imageSize.height() * scale));
......@@ -289,9 +295,9 @@ void ImageDocument::imageClicked(int x, int y)
m_shouldShrinkImage = !m_shouldShrinkImage;
if (m_shouldShrinkImage) {
windowSizeChanged(ScaleZoomedDocument);
windowSizeChanged();
} else {
restoreImageSize(ScaleZoomedDocument);
restoreImageSize();
updateStyleAndLayout();
......@@ -319,15 +325,15 @@ void ImageDocument::imageUpdated()
if (shouldShrinkToFit()) {
// Force resizing of the image
windowSizeChanged(ScaleOnlyUnzoomedDocument);
windowSizeChanged();
}
}
void ImageDocument::restoreImageSize(ScaleType type)
void ImageDocument::restoreImageSize()
{
DCHECK_EQ(m_shrinkToFitMode, Desktop);
if (!m_imageElement || !m_imageSizeIsKnown || m_imageElement->document() != this || (pageZoomFactor(this) < 1 && type == ScaleOnlyUnzoomedDocument))
if (!m_imageElement || !m_imageSizeIsKnown || m_imageElement->document() != this)
return;
DCHECK(m_imageElement->cachedImage());
......@@ -346,22 +352,10 @@ void ImageDocument::restoreImageSize(ScaleType type)
bool ImageDocument::imageFitsInWindow() const
{
DCHECK_EQ(m_shrinkToFitMode, Desktop);
if (!m_imageElement || m_imageElement->document() != this)
return true;
FrameView* view = frame()->view();
if (!view)
return true;
DCHECK(m_imageElement->cachedImage());
LayoutSize imageSize = m_imageElement->cachedImage()->imageSize(LayoutObject::shouldRespectImageOrientation(m_imageElement->layoutObject()), pageZoomFactor(this));
LayoutSize windowSize = LayoutSize(view->width(), view->height());
return imageSize.width() <= windowSize.width() && imageSize.height() <= windowSize.height();
return this->scale() >= 1;
}
void ImageDocument::windowSizeChanged(ScaleType type)
void ImageDocument::windowSizeChanged()
{
if (!m_imageElement || !m_imageSizeIsKnown || m_imageElement->document() != this)
return;
......@@ -393,13 +387,13 @@ void ImageDocument::windowSizeChanged(ScaleType type)
// If the window has been resized so that the image fits, restore the image size
// otherwise update the restored image size.
if (fitsInWindow)
restoreImageSize(type);
restoreImageSize();
else
resizeImageToFit(type);
resizeImageToFit();
} else {
// If the image isn't resized but needs to be, then resize it.
if (!fitsInWindow) {
resizeImageToFit(type);
resizeImageToFit();
m_didShrinkImage = true;
}
}
......@@ -434,7 +428,7 @@ DEFINE_TRACE(ImageDocument)
void ImageEventListener::handleEvent(ExecutionContext*, Event* event)
{
if (event->type() == EventTypeNames::resize) {
m_doc->windowSizeChanged(ImageDocument::ScaleOnlyUnzoomedDocument);
m_doc->windowSizeChanged();
} else if (event->type() == EventTypeNames::click && event->isMouseEvent()) {
MouseEvent* mouseEvent = toMouseEvent(event);
m_doc->imageClicked(mouseEvent->x(), mouseEvent->y());
......
......@@ -40,15 +40,10 @@ public:
return new ImageDocument(initializer);
}
enum ScaleType {
ScaleZoomedDocument,
ScaleOnlyUnzoomedDocument
};
ImageResource* cachedImage();
HTMLImageElement* imageElement() const { return m_imageElement.get(); }
void windowSizeChanged(ScaleType);
void windowSizeChanged();
void imageUpdated();
void imageClicked(int x, int y);
......@@ -62,10 +57,12 @@ private:
void createDocumentStructure();
// These methods are for m_shrinkToFitMode == Desktop.
void resizeImageToFit(ScaleType);
void restoreImageSize(ScaleType);
void resizeImageToFit();
void restoreImageSize();
bool imageFitsInWindow() const;
bool shouldShrinkToFit() const;
// Calculates the image size multiplier that's needed to fit the image to
// the window, taking into account page zoom and device scale.
float scale() const;
Member<HTMLImageElement> m_imageElement;
......
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "core/html/ImageDocument.h"
#include "core/dom/Document.h"
#include "core/dom/DocumentParser.h"
#include "core/loader/EmptyClients.h"
#include "core/testing/DummyPageHolder.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace blink {
namespace {
// An image of size 50x50.
Vector<unsigned char> jpegImage()
{
Vector<unsigned char> jpeg;
static const unsigned char data[] = {
0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10, 0x4a, 0x46, 0x49, 0x46, 0x00, 0x01, 0x01, 0x01, 0x00, 0x48,
0x00, 0x48, 0x00, 0x00, 0xff, 0xdb, 0x00, 0x43, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xdb, 0x00, 0x43, 0x01, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc0,
0x00, 0x11, 0x08, 0x00, 0x32, 0x00, 0x32, 0x03, 0x01, 0x22, 0x00, 0x02, 0x11, 0x01, 0x03, 0x11,
0x01, 0xff, 0xc4, 0x00, 0x14, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xc4, 0x00, 0x14, 0x10, 0x01, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xc4, 0x00,
0x15, 0x01, 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x02, 0xff, 0xc4, 0x00, 0x14, 0x11, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xda, 0x00, 0x0c, 0x03, 0x01,
0x00, 0x02, 0x11, 0x03, 0x11, 0x00, 0x3f, 0x00, 0x00, 0x94, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x03, 0xff, 0xd9
};
jpeg.append(data, sizeof(data));
return jpeg;
}
}
class WindowToViewportScalingChromeClient : public EmptyChromeClient {
public:
WindowToViewportScalingChromeClient()
: EmptyChromeClient()
, m_scaleFactor(1.f)
{
}
void setScalingFactor(float s) { m_scaleFactor = s; }
float windowToViewportScalar(const float s) const override { return s * m_scaleFactor; }
private:
float m_scaleFactor;
};
class ImageDocumentTest : public ::testing::Test {
protected:
void TearDown() override
{
ThreadState::current()->collectAllGarbage();
}
void createDocumentWithoutLoadingImage(int viewWidth, int viewHeight);
void createDocument(int viewWidth, int viewHeight);
void loadImage();
ImageDocument& document() const;
int imageWidth() const { return document().imageElement()->width(); }
int imageHeight() const { return document().imageElement()->height(); }
void setPageZoom(float);
void setWindowToViewportScalingFactor(float);
private:
Persistent<WindowToViewportScalingChromeClient> m_chromeClient;
std::unique_ptr<DummyPageHolder> m_dummyPageHolder;
};
void ImageDocumentTest::createDocumentWithoutLoadingImage(int viewWidth, int viewHeight)
{
Page::PageClients pageClients;
fillWithEmptyClients(pageClients);
m_chromeClient = new WindowToViewportScalingChromeClient();
pageClients.chromeClient = m_chromeClient;
m_dummyPageHolder = DummyPageHolder::create(IntSize(viewWidth, viewHeight), &pageClients);
LocalFrame& frame = m_dummyPageHolder->frame();
frame.document()->shutdown();
DocumentInit init(KURL(), &frame);
frame.localDOMWindow()->installNewDocument("image/jpeg", init);
}
void ImageDocumentTest::createDocument(int viewWidth, int viewHeight)
{
createDocumentWithoutLoadingImage(viewWidth, viewHeight);
loadImage();
}
ImageDocument& ImageDocumentTest::document() const
{
Document* document = m_dummyPageHolder->frame().domWindow()->document();
ImageDocument* imageDocument = static_cast<ImageDocument*>(document);
return *imageDocument;
}
void ImageDocumentTest::loadImage()
{
DocumentParser* parser = document().implicitOpen(ParserSynchronizationPolicy::ForceSynchronousParsing);
const Vector<unsigned char>& data = jpegImage();
parser->appendBytes(reinterpret_cast<const char*>(data.data()), data.size());
parser->finish();
}
void ImageDocumentTest::setPageZoom(float factor)
{
m_dummyPageHolder->frame().setPageZoomFactor(factor);
}
void ImageDocumentTest::setWindowToViewportScalingFactor(float factor)
{
m_chromeClient->setScalingFactor(factor);
}
TEST_F(ImageDocumentTest, ImageLoad)
{
createDocument(50, 50);
EXPECT_EQ(50, imageWidth());
EXPECT_EQ(50, imageHeight());
}
TEST_F(ImageDocumentTest, LargeImageScalesDown)
{
createDocument(25, 30);
EXPECT_EQ(25, imageWidth());
EXPECT_EQ(25, imageHeight());
createDocument(35, 20);
EXPECT_EQ(20, imageWidth());
EXPECT_EQ(20, imageHeight());
}
TEST_F(ImageDocumentTest, RestoreImageOnClick)
{
createDocument(30, 40);
document().imageClicked(4, 4);
EXPECT_EQ(50, imageWidth());
EXPECT_EQ(50, imageHeight());
}
TEST_F(ImageDocumentTest, InitialZoomDoesNotAffectScreenFit)
{
createDocumentWithoutLoadingImage(20, 10);
setPageZoom(2.f);
loadImage();
EXPECT_EQ(10, imageWidth());
EXPECT_EQ(10, imageHeight());
document().imageClicked(4, 4);
EXPECT_EQ(50, imageWidth());
EXPECT_EQ(50, imageHeight());
}
TEST_F(ImageDocumentTest, ZoomingDoesNotChangeRelativeSize)
{
createDocument(75, 75);
setPageZoom(0.5f);
document().windowSizeChanged();
EXPECT_EQ(50, imageWidth());
EXPECT_EQ(50, imageHeight());
setPageZoom(2.f);
document().windowSizeChanged();
EXPECT_EQ(50, imageWidth());
EXPECT_EQ(50, imageHeight());
}
TEST_F(ImageDocumentTest, ImageScalesDownWithDsf)
{
createDocumentWithoutLoadingImage(20, 30);
setWindowToViewportScalingFactor(2.f);
loadImage();
EXPECT_EQ(10, imageWidth());
EXPECT_EQ(10, imageHeight());
}
} // namespace blink
......@@ -78,7 +78,7 @@ DummyPageHolder::DummyPageHolder(
if (!m_frameLoaderClient)
m_frameLoaderClient = EmptyFrameLoaderClient::create();
m_frame = LocalFrame::create(m_frameLoaderClient.get(), &m_page->frameHost(), 0);
m_frame = LocalFrame::create(m_frameLoaderClient.get(), &m_page->frameHost(), nullptr);
m_frame->setView(FrameView::create(m_frame.get(), initialViewSize));
m_frame->view()->page()->frameHost().visualViewport().setSize(initialViewSize);
m_frame->init();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment