Skip to content

Commit

Permalink
[srcset] Add a max DPR value
Browse files Browse the repository at this point in the history
PSA: https://groups.google.com/a/chromium.org/d/msg/blink-dev/sIGNgOAC0oc/TFST-dJxAwAJ

When srcset selects images, it takes the screen density into account,
but beyond a certain density, that can result in excessive downloaded
images.

Currently, when adding `w` descriptors to srcset, developers run a
risk that large `w` descriptor images will be used on small screens
with very-high density values.

For example, when defining
`<img srcset="300.jpg 300w,
              600.jpg 600w,
              1200.jpg 1200w,
              2400.jpg 2400w">`,
developers that define the 2400px wide image typically intend for it
to be used for very large screens (e.g. a 32" 2x screen), but it often
ends up also being used on small screens (e.g. a phone with 4x DPR and
viewport width of 600+px) .

Research[1] shows that density above 2-2.2x is not visible by most
people, and therefore we could avoid wasting those bytes and avoid
the related slowdown.

See whatwg/html#4421 for full discussion

Change-Id: I52afd505b023767883088a939083f9f835e7fa14

BUG: 1125973

[1] https://observablehq.com/@eeeps/visual-acuity-and-device-pixel-ratio

Change-Id: I52afd505b023767883088a939083f9f835e7fa14
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2395619
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812698}
  • Loading branch information
Yoav Weiss authored and Commit Bot committed Oct 1, 2020
1 parent 1d00d6d commit 06f97a0
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 0 deletions.
11 changes: 11 additions & 0 deletions third_party/blink/renderer/core/html/parser/html_srcset_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "third_party/blink/renderer/platform/json/json_values.h"
#include "third_party/blink/renderer/platform/loader/fetch/memory_cache.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
#include "third_party/blink/renderer/platform/wtf/text/character_visitor.h"
#include "third_party/blink/renderer/platform/wtf/text/parsing_utilities.h"
#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
Expand Down Expand Up @@ -425,10 +426,20 @@ static ImageCandidate PickBestImageCandidate(
Vector<ImageCandidate>& image_candidates,
Document* document = nullptr) {
const float kDefaultDensityValue = 1.0;
// The srcset image source selection mechanism is user-agent specific:
// https://html.spec.whatwg.org/multipage/images.html#selecting-an-image-source
//
// Setting max density value based on https://github.com/whatwg/html/pull/5901
const float kMaxDensity = 2.2;
bool ignore_src = false;
if (image_candidates.IsEmpty())
return ImageCandidate();

if (RuntimeEnabledFeatures::SrcsetMaxDensityEnabled() &&
device_scale_factor > kMaxDensity) {
device_scale_factor = kMaxDensity;
}

// http://picture.responsiveimages.org/#normalize-source-densities
for (ImageCandidate& image : image_candidates) {
if (image.GetResourceWidth() > 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/platform/web_network_state_notifier.h"
#include "third_party/blink/renderer/platform/runtime_enabled_features.h"

namespace blink {

Expand Down Expand Up @@ -323,4 +324,25 @@ TEST(HTMLSrcsetParserTest, SaveDataEnabledBasic) {
}
}

TEST(HTMLSrcsetParserTest, MaxDensityEnabled) {
RuntimeEnabledFeatures::SetSrcsetMaxDensityEnabled(true);
SrcsetParserTestCase test_cases[] = {
{10.0, -1, "src.gif", "2x.gif 2e1x", "src.gif", 1.0, -1},
{2.5, -1, "src.gif", "1.5x.gif 1.5x, 3x.gif 3x", "3x.gif", 3.0, -1},
{4.0, 400, "", "400.gif 400w, 1000.gif 1000w", "1000.gif", 2.5, 1000},
{0, 0, nullptr, nullptr, nullptr,
0} // Do not remove the terminator line.
};

for (unsigned i = 0; test_cases[i].src_input; ++i) {
SrcsetParserTestCase test = test_cases[i];
ImageCandidate candidate = BestFitSourceForImageAttributes(
test.device_scale_factor, test.effective_size, test.src_input,
test.srcset_input);
ASSERT_EQ(test.output_density, candidate.Density());
ASSERT_EQ(test.output_resource_width, candidate.GetResourceWidth());
ASSERT_EQ(test.output_url, candidate.ToString().Ascii());
}
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -1784,6 +1784,9 @@
name: "SmsReceiver",
status: {"default": "experimental", "Android": "stable"},
},
{
name: "SrcsetMaxDensity",
},
// Used as argument in attribute of stable-release functions/interfaces
// where a runtime-enabled feature name is required for correct IDL syntax.
// This is a global flag; do not change its status.
Expand Down

0 comments on commit 06f97a0

Please sign in to comment.