diff --git a/README.md b/README.md index c168809..cc62f2e 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,13 @@ width in pixels of the associated image and must have the suffix attribute of the image. This is the image that will be displayed by browsers that do not support the `srcset` syntax. +If a derivative image ends up narrower than its width description — for +example, because the original image is only 1024 pixels wide and upscaling is +disabled — *Image Process* replaces the description in the generated `srcset` +with the actual width of the image (e.g., `1024w` instead of `1600w`). Entries +that would duplicate an already listed width are omitted, so the browser is +never offered several copies of the same image. + Both definitions above also demonstrate how the input image may be transcoded into another file format. This allows you to transcode your image from, for example, a `png` original to `webp` diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000..2316c92 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,3 @@ +Release type: minor + +Use actual image widths for `w` descriptors in generated `srcset` attributes, and omit entries that would duplicate an existing width, when a derivative image is narrower than declared (e.g., because the source image is too small to be scaled up) diff --git a/pelican/plugins/image_process/image_process.py b/pelican/plugins/image_process/image_process.py index 5891605..ada3deb 100644 --- a/pelican/plugins/image_process/image_process.py +++ b/pelican/plugins/image_process/image_process.py @@ -531,6 +531,35 @@ def format_srcset_element(path, condition): return f"{path} {condition}" +def get_srcset_descriptor(condition, image_size, seen_widths): + """Return the srcset descriptor matching the size of the processed image. + + A width descriptor (e.g. "3000w") is only accurate if the processed image + is actually that wide. When the source image is too small (and upscaling + is disabled), the derivative ends up narrower than declared. In that case + the descriptor is replaced by the real width of the image, and the entry + is dropped (None is returned) if another entry already has that width. + + seen_widths is a set of widths already used in the srcset; it is updated + in place. + """ + match = re.fullmatch(r"(\d+)w", condition.strip()) + if match is None or image_size is None: + return condition + + declared_width = int(match[1]) + width = image_size[0] + if width >= declared_width: + seen_widths.add(declared_width) + return condition + + if width in seen_widths: + return None + + seen_widths.add(width) + return f"{width}w" + + def build_srcset(img, settings, derivative): path = compute_paths(img["src"], settings, derivative) process = settings["IMAGE_PROCESS"][derivative] @@ -572,13 +601,16 @@ def build_srcset(img, settings, derivative): img["sizes"] = process["sizes"] srcset = [] + seen_widths = set() for src in process["srcset"]: entry_format = get_target_format(src, top_default_format) filename = get_target_filename(path.filename, entry_format) file_path = posixpath.join(path.base_url, src[0], filename) - srcset.append(format_srcset_element(file_path, src[0])) destination = os.path.join(str(path.base_path), src[0], filename) - process_image((path.source, destination, src[1]), settings) + image_size = process_image((path.source, destination, src[1]), settings) + descriptor = get_srcset_descriptor(src[0], image_size, seen_widths) + if descriptor is not None: + srcset.append(format_srcset_element(file_path, descriptor)) if len(srcset) > 0: img["srcset"] = ", ".join(srcset) @@ -620,15 +652,18 @@ def construct_picture_tag(soup, img, sources, settings): top_source_format = get_target_format(s) srcset = [] + seen_widths = set() for src in s["srcset"]: entry_format = get_target_format(src, top_source_format) filename = get_target_filename(s["filename"], entry_format) url = os.path.join(s["base_url"], s["name"], src[0], filename) - srcset.append(format_srcset_element(str(url), src[0])) source = os.path.join(settings["PATH"], s["url"][1:]) destination = os.path.join(s["base_path"], s["name"], src[0], filename) - process_image((source, destination, src[1]), settings) + image_size = process_image((source, destination, src[1]), settings) + descriptor = get_srcset_descriptor(src[0], image_size, seen_widths) + if descriptor is not None: + srcset.append(format_srcset_element(str(url), descriptor)) if len(srcset) > 0: source_tag["srcset"] = ", ".join(srcset) @@ -722,15 +757,18 @@ def generate_srcset_and_insert_source(img, s, settings): top_source_format = get_target_format(s) srcset = [] + seen_widths = set() for src in s["srcset"]: entry_format = get_target_format(src, top_source_format) filename = get_target_filename(s["filename"], entry_format) url = posixpath.join(s["base_url"], s["name"], src[0], filename) - srcset.append(format_srcset_element(str(url), src[0])) source = os.path.join(settings["PATH"], s["url"][1:]) destination = os.path.join(s["base_path"], s["name"], src[0], filename) - process_image((source, destination, src[1]), settings) + image_size = process_image((source, destination, src[1]), settings) + descriptor = get_srcset_descriptor(src[0], image_size, seen_widths) + if descriptor is not None: + srcset.append(format_srcset_element(str(url), descriptor)) if len(srcset) > 0: # Append source elements to the picture in the same order diff --git a/pelican/plugins/image_process/test_image_process.py b/pelican/plugins/image_process/test_image_process.py index 89548ae..4d32105 100644 --- a/pelican/plugins/image_process/test_image_process.py +++ b/pelican/plugins/image_process/test_image_process.py @@ -12,6 +12,7 @@ from pelican.plugins.image_process import ( ExifTool, compute_paths, + get_srcset_descriptor, get_target_filename, harvest_images_in_fragment, process_image, @@ -638,6 +639,95 @@ def test_path_normalization(mocker, orig_src, orig_img, new_src, new_img): } +def mock_process_image_size(image, settings): + """Mock process_image(), pretending derivatives have their declared width.""" + descriptor = Path(image[1]).parent.name + if descriptor.endswith("w") and descriptor[:-1].isdigit(): + return int(descriptor[:-1]), 384 + return 512, 384 + + +@pytest.mark.parametrize( + "condition, image_size, seen_widths, expected, expected_seen", + [ + # Density descriptors are left untouched. + ("2x", (100, 100), set(), "2x", set()), + # Image could not be processed: keep declared descriptor. + ("800w", None, set(), "800w", set()), + # Image is (at least) as wide as declared. + ("800w", (800, 600), set(), "800w", {800}), + ("800w", (900, 600), set(), "800w", {800}), + # Image is narrower than declared: use the actual width. + ("2000w", (1500, 1000), {1000}, "1500w", {1000, 1500}), + # Image is narrower than declared and that width is already used. + ("3000w", (1500, 1000), {1000, 1500}, None, {1000, 1500}), + ], +) +def test_get_srcset_descriptor( + condition, image_size, seen_widths, expected, expected_seen +): + assert get_srcset_descriptor(condition, image_size, seen_widths) == expected + assert seen_widths == expected_seen + + +def test_srcset_does_not_advertise_upscaled_widths(tmp_path): + """Entries wider than the source image are not advertised in srcset. + + Related to issue #53 https://github.com/pelican-plugins/image-process/issues/53 + """ + settings = get_settings( + PATH=TEST_DATA, + OUTPUT_PATH=tmp_path, + IMAGE_PROCESS={ + "large": { + "type": "responsive-image", + "srcset": [ + ("600w", ["scale_out 600 1 False"]), + ("1000w", ["scale_out 1000 1 False"]), + ("2000w", ["scale_out 2000 1 False"]), + ("3000w", ["scale_out 3000 1 False"]), + ], + "default": "1000w", + }, + }, + IMAGE_PROCESS_DIR="derivs", + ) + + # pelican-bird.jpg is 1024px wide. + tag = '' + soup = BeautifulSoup(harvest_images_in_fragment(tag, settings), "html.parser") + + assert soup.img["srcset"] == ( + "/derivs/large/600w/pelican-bird.jpg 600w, " + "/derivs/large/1000w/pelican-bird.jpg 1000w, " + "/derivs/large/2000w/pelican-bird.jpg 1024w" + ) + + +def test_picture_srcset_does_not_advertise_upscaled_widths(mocker): + """Entries wider than the source image are not advertised in .""" + process = mocker.patch("pelican.plugins.image_process.image_process.process_image") + # The source image is 1024px wide; scaled derivatives cannot be wider. + process.side_effect = lambda image, settings: ( + min(mock_process_image_size(image, settings)[0], 1024), + 768, + ) + + settings = get_settings( + IMAGE_PROCESS=COMPLEX_TRANSFORMS, IMAGE_PROCESS_DIR="derivs" + ) + tag = ( + '' + '' + ) + soup = BeautifulSoup(harvest_images_in_fragment(tag, settings), "html.parser") + + assert soup.find("source")["srcset"] == ( + "/images/derivs/pict/default/640w/pelican.jpg 640w, " + "/images/derivs/pict/default/1024w/pelican.jpg 1024w" + ) + + @pytest.mark.parametrize( "orig_tag, new_tag, call_args", [ @@ -849,7 +939,7 @@ def test_path_normalization(mocker, orig_src, orig_img, new_src, new_img): def test_html_and_pictures_generation(mocker, orig_tag, new_tag, call_args): """Tests that the generated html is as expected and the images are processed.""" process = mocker.patch("pelican.plugins.image_process.image_process.process_image") - process.return_value = (512, 384) + process.side_effect = mock_process_image_size settings = get_settings( IMAGE_PROCESS=COMPLEX_TRANSFORMS, IMAGE_PROCESS_DIR="derivs" @@ -997,7 +1087,7 @@ def test_special_chars_in_image_path_are_handled_properly(mocker, orig_tag, new_ Related to issue #78 https://github.com/pelican-plugins/image-process/issues/78 """ process = mocker.patch("pelican.plugins.image_process.image_process.process_image") - process.return_value = (512, 384) + process.side_effect = mock_process_image_size settings = get_settings( IMAGE_PROCESS=COMPLEX_TRANSFORMS, IMAGE_PROCESS_DIR="derivs"