Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Take min/max-height into account in get_min_max_width for images #2740

Merged
merged 2 commits into from Jan 18, 2022

Conversation

Mellthas
Copy link
Member

@Mellthas Mellthas commented Jan 16, 2022

This includes a rework of the size calculations for images, so that min/max constraints are always honored. Previously, constraints were ignored in some cases to maintain aspect ratio. See the last part of the section on min/max widths in https://www.w3.org/TR/CSS21/visudet.html#min-max-widths for the spec on the matter.

The first commit is a small change to enable rendering of zero-sized image frames. This ensures that outlines are drawn and that the image is still available as link target.

Fixes #2738

Make sure to always render image decorations and check for named
destinations. Only skip rendering the actual image if content width or
height is 0.
@Mellthas
Copy link
Member Author

Test cases for the reworked size calculations

HTML
<!DOCTYPE html>
<html>

<head>
<meta charset="UTF-8">
<style>
    @page {
        size: 600pt 900pt;
        margin: 30pt;
    }

    img {
        vertical-align: top;
    }

    pre {
        display: inline-block;
        margin: 0;
        width: 200pt;
        font-size: 10pt;
        vertical-align: top;
    }

    .inline-block {
        display: inline-block;
        vertical-align: top;
        border: thin solid;
    }

    .break {
        page-break-before: always;
    }
</style>
</head>

<body>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: 100px; min-height: 200px; max-width: 200px;">
</span>
<pre>
width: auto;
height: 100px;
min-height: 200px;
max-width: 200px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: 100px; height: auto; min-width: 200px; max-height: 200px;">
</span>
<pre>
width: 100px;
height: auto;
min-width: 200px;
max-height: 200px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 200px; max-height: 200px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 200px;
max-height: 200px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 200px; min-height: 200px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 200px;
min-height: 200px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 200px; min-height: 180px; max-height: 200px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 200px;
min-height: 180px;
max-height: 200px;
</pre><br>



<div class="break"></div>
<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 100px; min-height: 200px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 100px;
min-height: 200px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; min-width: 200px; max-height: 100px;">
</span>
<pre>
width: auto;
height: auto;
min-width: 200px;
max-height: 100px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 200px; min-height: 100px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 200px;
min-height: 100px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; min-width: 100px; max-height: 200px;">
</span>
<pre>
width: auto;
height: auto;
min-width: 100px;
max-height: 200px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 100px; max-height: 200px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 100px;
max-height: 200px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 100px; max-height: 100px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 100px;
max-height: 100px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 100px; max-height: 50px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 100px;
max-height: 50px;
</pre><br>

<span class="inline-block">
    <img src="http://placekitten.com/500/300" style="width: auto; height: auto; max-width: 250px; max-height: 200px;">
</span>
<pre>
width: auto;
height: auto;
max-width: 150px;
max-height: 100px;
</pre><br>

</body>

</html>

Output for comparison:
image-min-max-1.0.2.pdf
image-min-max-before.pdf
image-min-max-after.pdf

This includes a rework of the size calculations for images, so that
min/max constraints are always honored. Previously, constraints were
ignored in some cases to maintain aspect ratio. See the last part of
the section on min/max widths in [1] for the spec on the matter.

TODO: While the containing block is not set yet on the frame during
min/max-width determination, it can already be determined in some cases
due to fixed dimensions on the ancestor forming the containing block.
In such cases, percentage values could be resolved already. This needs
more consideration, e.g. in regards to min/max dimensions on the parent
and percentages, which could necessitate checking further ancestors.
Maybe `get_min_max_width` could receive preliminary containing-block
dimensions, which would still be undefined if an `auto` width/height is
encountered.

[1] https://www.w3.org/TR/CSS21/visudet.html#min-max-widths

Fixes #2738
@Mellthas Mellthas merged commit b32fdbf into master Jan 18, 2022
@Mellthas Mellthas deleted the image-size branch January 18, 2022 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: max-height on image in table breaks second column
2 participants