Bug 1149357: Make nsImageFrame::mIntrinsicSize account for density. r=dholbert
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 16 May 2018 19:39:16 +0200
changeset 474077 d19bef57314b5cfcc1e692be514fe2597d08c7ae
parent 474076 d7c126843dc0b7cc7251bbf7c21c954d3f391c49
child 474078 2d94b092d7e0dd61f7d0e1cf8ff4f8c7dd2e3053
push id9374
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:43:20 +0000
treeherdermozilla-beta@160e085dfb0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1149357, 215083
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1149357: Make nsImageFrame::mIntrinsicSize account for density. r=dholbert Only doing it in ComputeSize (via GetNaturalSize) is unsound, and the rest of the users of mIntrinsicSize definitely do need scaling accounted for. Move the adjustment to nsImageFrame for two reasons: * Prevents adding more dependencies from nsIImageLoadingContent, which otherwise would need to go away anyway in bug 215083. * Avoids having to duplicate the image orientation logic, since mImage is already an OrientedImage if needed. MozReview-Commit-ID: EA0n0TctZhN
dom/html/HTMLImageElement.h
layout/generic/nsImageFrame.cpp
layout/reftests/image/image-srcset-isize-ref.html
layout/reftests/image/image-srcset-isize.html
layout/reftests/image/reftest.list
--- a/dom/html/HTMLImageElement.h
+++ b/dom/html/HTMLImageElement.h
@@ -38,16 +38,21 @@ public:
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(HTMLImageElement,
                                            nsGenericHTMLElement)
 
   // nsISupports
   NS_DECL_ISUPPORTS_INHERITED
 
   virtual bool Draggable() const override;
 
+  ResponsiveImageSelector* GetResponsiveImageSelector()
+  {
+    return mResponsiveSelector.get();
+  }
+
   // Element
   virtual bool IsInteractiveHTMLContent(bool aIgnoreTabindex) const override;
 
   // EventTarget
   virtual void AsyncEventRunning(AsyncEventDispatcher* aEvent) override;
 
   NS_IMPL_FROMNODE_HTML_WITH_TAG(HTMLImageElement, img)
 
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -13,16 +13,18 @@
 #include "gfxUtils.h"
 #include "mozilla/ComputedStyle.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Encoding.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/gfx/Helpers.h"
 #include "mozilla/gfx/PathHelpers.h"
+#include "mozilla/dom/HTMLImageElement.h"
+#include "mozilla/dom/ResponsiveImageSelector.h"
 #include "mozilla/layers/WebRenderLayerManager.h"
 #include "mozilla/MouseEvents.h"
 #include "mozilla/Unused.h"
 
 #include "nsCOMPtr.h"
 #include "nsFontMetrics.h"
 #include "nsIImageLoadingContent.h"
 #include "nsString.h"
@@ -289,29 +291,57 @@ nsImageFrame::Init(nsIContent*       aCo
     if (!HaveSpecifiedSize(StylePosition())) {
       categoryToBoostPriority |= imgIRequest::CATEGORY_SIZE_QUERY;
     }
 
     currentRequest->BoostPriority(categoryToBoostPriority);
   }
 }
 
+static void
+ScaleIntrinsicSizeForDensity(nsIContent& aContent, nsSize& aSize)
+{
+  auto* image = HTMLImageElement::FromNode(aContent);
+  if (!image) {
+    return;
+  }
+
+  ResponsiveImageSelector* selector = image->GetResponsiveImageSelector();
+  if (!selector) {
+    return;
+  }
+
+  double density = selector->GetSelectedImageDensity();
+  MOZ_ASSERT(density > 0.0);
+  if (density == 1.0) {
+    return;
+  }
+
+  if (aSize.width != -1) {
+    aSize.width = NSToCoordRound(double(aSize.width) / density);
+  }
+  if (aSize.height != -1) {
+    aSize.height = NSToCoordRound(double(aSize.height) / density);
+  }
+}
+
 bool
 nsImageFrame::UpdateIntrinsicSize(imgIContainer* aImage)
 {
   MOZ_ASSERT(aImage, "null image");
   if (!aImage)
     return false;
 
   IntrinsicSize oldIntrinsicSize = mIntrinsicSize;
   mIntrinsicSize = IntrinsicSize();
 
   // Set intrinsic size to match aImage's reported intrinsic width & height.
   nsSize intrinsicSize;
   if (NS_SUCCEEDED(aImage->GetIntrinsicSize(&intrinsicSize))) {
+    ScaleIntrinsicSizeForDensity(*mContent, intrinsicSize);
     // If the image has no intrinsic width, intrinsicSize.width will be -1, and
     // we can leave mIntrinsicSize.width at its default value of eStyleUnit_None.
     // Otherwise we use intrinsicSize.width. Height works the same way.
     if (intrinsicSize.width != -1)
       mIntrinsicSize.width.SetCoordValue(intrinsicSize.width);
     if (intrinsicSize.height != -1)
       mIntrinsicSize.height.SetCoordValue(intrinsicSize.height);
   } else {
@@ -854,56 +884,18 @@ nsImageFrame::ComputeSize(gfxContext *aR
                           const LogicalSize& aCBSize,
                           nscoord aAvailableISize,
                           const LogicalSize& aMargin,
                           const LogicalSize& aBorder,
                           const LogicalSize& aPadding,
                           ComputeSizeFlags aFlags)
 {
   EnsureIntrinsicSizeAndRatio();
-
-  nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mContent);
-  NS_ASSERTION(imageLoader, "No content node??");
-  mozilla::IntrinsicSize intrinsicSize(mIntrinsicSize);
-
-  // XXX(seth): We may sometimes find ourselves in the situation where we have
-  // mImage, but imageLoader's current request does not have a size yet.
-  // This can happen when we load an image speculatively from cache, it fails
-  // to validate, and the new image load hasn't fired SIZE_AVAILABLE yet. In
-  // this situation we should always use mIntrinsicSize, because
-  // GetNaturalWidth/Height will return 0, so we check CurrentRequestHasSize()
-  // below. See bug 1019840. We will fix this in bug 1141395.
-
-  // Content may override our default dimensions. This is termed as overriding
-  // the intrinsic size by the spec, but all other consumers of mIntrinsic*
-  // values are being used to refer to the real/true size of the image data.
-  if (imageLoader && imageLoader->CurrentRequestHasSize() && mImage &&
-      intrinsicSize.width.GetUnit() == eStyleUnit_Coord &&
-      intrinsicSize.height.GetUnit() == eStyleUnit_Coord) {
-    uint32_t width;
-    uint32_t height;
-    if (NS_SUCCEEDED(imageLoader->GetNaturalWidth(&width)) &&
-        NS_SUCCEEDED(imageLoader->GetNaturalHeight(&height))) {
-      nscoord appWidth = nsPresContext::CSSPixelsToAppUnits((int32_t)width);
-      nscoord appHeight = nsPresContext::CSSPixelsToAppUnits((int32_t)height);
-      // If this image is rotated, we'll need to transpose the natural
-      // width/height.
-      bool coordFlip;
-      if (StyleVisibility()->mImageOrientation.IsFromImage()) {
-        coordFlip = mImage->GetOrientation().SwapsWidthAndHeight();
-      } else {
-        coordFlip = StyleVisibility()->mImageOrientation.SwapsWidthAndHeight();
-      }
-      intrinsicSize.width.SetCoordValue(coordFlip ? appHeight : appWidth);
-      intrinsicSize.height.SetCoordValue(coordFlip ? appWidth : appHeight);
-    }
-  }
-
   return ComputeSizeWithIntrinsicDimensions(aRenderingContext, aWM,
-                                            intrinsicSize, mIntrinsicRatio,
+                                            mIntrinsicSize, mIntrinsicRatio,
                                             aCBSize, aMargin, aBorder, aPadding,
                                             aFlags);
 }
 
 // XXXdholbert This function's clients should probably just be calling
 // GetContentRectRelativeToSelf() directly.
 nsRect
 nsImageFrame::GetInnerArea() const
new file mode 100644
--- /dev/null
+++ b/layout/reftests/image/image-srcset-isize-ref.html
@@ -0,0 +1,25 @@
+<!doctype html>
+<html reftest-zoom="2">
+<title>CSS Test Reference</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<style>
+.image-container {
+  display: inline;
+}
+.content-container {
+  display: inline-block;
+}
+.flex-container {
+  align-items: center;
+  display: flex;
+}
+</style>
+<div class="flex-container">
+  <div class="image-container">
+    <img src="200.png" width="100">
+  </div>
+  <div class="content-container">
+    Should see me right by the side of the image.
+  </div>
+</div>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/image/image-srcset-isize.html
@@ -0,0 +1,41 @@
+<!doctype html>
+<html reftest-zoom="2" class="reftest-wait">
+<title>CSS Test: srcset intrinsic size isn't confused</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1149357">
+<style>
+.image-container {
+  display: inline;
+}
+.content-container {
+  display: inline-block;
+}
+.flex-container {
+  align-items: center;
+  display: flex;
+}
+</style>
+<script>
+  // reftest-zoom is only applied at onload, so ensure the source-selection
+  // has happened after that
+  function clearWait() {
+    document.documentElement.classList.remove("reftest-wait");
+  }
+  window.addEventListener("load", function() {
+    setTimeout(function() {
+      var img = document.querySelector("img");
+      img.onload = clearWait;
+      img.onerror = clearWait;
+      img.src = img.src;
+    }, 0);
+  });
+</script>
+<div class="flex-container">
+  <div class="image-container">
+    <img srcset="50.png 0.5x, 100.png 1x, 200.png 2x, 300.png 3x, 400.png">
+  </div>
+  <div class="content-container">
+    Should see me right by the side of the image.
+  </div>
+</div>
+</html>
--- a/layout/reftests/image/reftest.list
+++ b/layout/reftests/image/reftest.list
@@ -113,16 +113,17 @@ fuzzy(1,1) == image-orientation-backgrou
 == image-srcset-basic-selection-width-0.5x.html image-srcset-basic-selection-width-0.5x-ref.html
 == image-srcset-basic-selection-width-10x.html image-srcset-basic-selection-width-10x-ref.html
 == image-srcset-basic-selection-width-2x.html image-srcset-basic-selection-width-2x-ref.html
 == image-srcset-basic-selection-width-1x.html image-srcset-basic-selection-width-1x-ref.html
 == image-srcset-default-2x.html image-srcset-default-2x-ref.html
 == image-srcset-default-1x.html image-srcset-default-1x-ref.html
 == image-srcset-default-src-2x.html image-srcset-default-src-2x-ref.html
 == image-srcset-default-src-1x.html image-srcset-default-src-1x-ref.html
+== image-srcset-isize.html image-srcset-isize-ref.html
 == image-srcset-orientation-2x.html image-srcset-orientation-2x-ref.html
 == image-srcset-orientation-1x.html image-srcset-orientation-1x-ref.html
 == image-srcset-svg-3x.html image-srcset-svg-3x-ref.html
 == image-srcset-svg-2x.html image-srcset-svg-2x-ref.html
 == image-srcset-svg-1x.html image-srcset-svg-1x-ref.html
 == image-srcset-svg-default-2x.html image-srcset-svg-default-2x-ref.html
 == image-srcset-svg-default-1x.html image-srcset-svg-default-1x-ref.html