Bug 1149357: Make mIntrinsicSize account for density. r?dholbert draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 16 May 2018 19:39:16 +0200
changeset 796237 b90be48dcc97066e106fa1777c3d3acc1e46cd1d
parent 796087 1800b8895c08bc0c60302775dc0a4b5ea4deb310
child 796729 81868c3cee21a54522fdbac79c857390402a87b2
push id110188
push userbmo:emilio@crisal.io
push dateThu, 17 May 2018 09:39:34 +0000
reviewersdholbert
bugs1149357, 215083
milestone62.0a1
Bug 1149357: Make 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,53 @@ nsImageFrame::Init(nsIContent*       aCo
     if (!HaveSpecifiedSize(StylePosition())) {
       categoryToBoostPriority |= imgIRequest::CATEGORY_SIZE_QUERY;
     }
 
     currentRequest->BoostPriority(categoryToBoostPriority);
   }
 }
 
+static void
+AdjustIntrinsicSizeIfNeeded(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 (aSize.width != -1) {
+    aSize.width = NSToIntRound(double(aSize.width) / density);
+  }
+  if (aSize.height != -1) {
+    aSize.height = NSToIntRound(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))) {
+    AdjustIntrinsicSizeIfNeeded(*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 +880,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
@@ -121,12 +121,14 @@ fuzzy(1,1) == image-orientation-backgrou
 == 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
 
+== image-srcset-isize.html image-srcset-isize-ref.html
+
 == image-resize-percent-height.html image-resize-ref.html
 == image-resize-percent-width.html image-resize-ref.html
 
 == moz-broken-matching-1.html moz-broken-matching-1-ref.html