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

Fix image height content fit #9116

Merged
merged 17 commits into from
Aug 21, 2024
Merged

Fix image height content fit #9116

merged 17 commits into from
Aug 21, 2024

Conversation

hannahblair
Copy link
Collaborator

@hannahblair hannahblair commented Aug 14, 2024

Description

Adds full height to the image wrapper to ensure that long images fit the container.

Closes: #9080

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Tests

  1. PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests: bash scripts/run_all_tests.sh

  2. You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Aug 14, 2024

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Storybook ready! Storybook preview
🦄 Changes detecting...

Install Gradio from this PR

pip install https://gradio-pypi-previews.s3.amazonaws.com/03cd6d5066bc31c1ce8d5786e6e526fba46a7b44/gradio-4.41.0-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@03cd6d5066bc31c1ce8d5786e6e526fba46a7b44#subdirectory=client/python"

Install Gradio JS Client from this PR

npm install https://gradio-npm-previews.s3.amazonaws.com/03cd6d5066bc31c1ce8d5786e6e526fba46a7b44/gradio-client-1.5.0.tgz

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Aug 14, 2024

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
@gradio/image patch
@gradio/simpleimage patch
@gradio/storybook patch
gradio patch
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Fix image height content fit

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

Sorry, something went wrong.

Copy link
Collaborator

@aliabid94 aliabid94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Noticed this also stretches out images smaller than the height or width of the container to fit the container now. Not sure if that is desired or not.

Copy link
Collaborator

@aliabid94 aliabid94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Agree that newlines should be supported, either by replacing '\n' with <br>, or supporting any markdown in the placeholder.

@abidlabs
Copy link
Member

Noticed this also stretches out images smaller than the height or width of the container to fit the container now

Hmm I don't think this is intended, right @hannahblair?

hannahblair and others added 3 commits August 16, 2024 13:23
@hannahblair
Copy link
Collaborator Author

That's a good point @aliabid94! We probably don't want that. I've fixed that issue in this PR using object-fit: scale-down which combines contain and none, which will ensure that large images will fit the container, but small images won't be scaled up to fit the container. This probably happens in other components too, so maybe we can change those to use scale-down in another PR.

@aliabid94
Copy link
Collaborator

Hmm now I've noticed that a standalone gr.Image takes up much more vertical space. Here's my demo:

import gradio as gr
from PIL import Image, ImageDraw


with gr.Blocks() as demo:
    image = gr.Image()

    with gr.Row():
        width = gr.Slider(100, 1000, 100, label="Width")
        height = gr.Slider(100, 1000, 100, label="Height")

    @gr.on(triggers=[width.change, height.change, demo.load], inputs=[width, height], outputs=image)
    def update_image(width, height):
        img = Image.new('RGB', (width, height), color='white')
        draw = ImageDraw.Draw(img)
        draw.rectangle(
            [(0, 0), (width - 1, height - 1)],
            outline='red',
            width=5
        )
        return img

if __name__ == "__main__":
    demo.launch()

Here's what this looks like on main:

Screenshot 2024-08-19 at 12 12 14 PM

On this PR:

Screenshot 2024-08-19 at 12 12 49 PM

@hannahblair
Copy link
Collaborator Author

hannahblair commented Aug 20, 2024

Oh, this is quite a tricky one. Let me rethink this! Nice demo @aliabid94

@hannahblair
Copy link
Collaborator Author

@aliabid94, thanks for reviewing that - I think we're there now 😅

@aliabid94
Copy link
Collaborator

Ran the demo with the heoght fixed to 500px, code below. If the image is taller than 500px it is getting cut off.

Screenshot 2024-08-20 at 10 34 12 AM
import gradio as gr
from PIL import Image, ImageDraw


with gr.Blocks() as demo:
    image = gr.Image(height="500px")

    with gr.Row():
        width = gr.Slider(100, 1000, 100, label="Width")
        height = gr.Slider(100, 1000, 100, label="Height")

    @gr.on(triggers=[width.change, height.change, demo.load], inputs=[width, height], outputs=image)
    def update_image(width, height):
        img = Image.new('RGB', (width, height), color='white')
        draw = ImageDraw.Draw(img)
        draw.rectangle(
            [(0, 0), (width - 1, height - 1)],
            outline='red',
            width=5
        )
        return img

if __name__ == "__main__":
    demo.launch()

@aliabid94
Copy link
Collaborator

Still see the massive image when the height is unfixed:
Screenshot 2024-08-20 at 4 35 14 PM

@whitphx
Copy link
Member

whitphx commented Aug 21, 2024

I pushed a patch as a suggestion but its ImagePreview styling strategy became different from ImageUpload's one in that ImagePreview's img is max-width: 100%; max-height: 100% while ImageUpload's img is width: 100%; height: 100%; object-fit: scale-down.
Will consider of another impl for consistency.

Done↓

Fix
hannahblair and others added 4 commits August 21, 2024 10:51
Copy link
Collaborator

@aliabid94 aliabid94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works perfectly, thanks @hannahblair!

@hannahblair
Copy link
Collaborator Author

Works perfectly, thanks @hannahblair!

Thanks for your patience with it! 😅 The fix was all @whitphx 🚀

@hannahblair hannahblair merged commit ba6322e into main Aug 21, 2024
21 of 22 checks passed
@hannahblair hannahblair deleted the image-height-fit branch August 21, 2024 17:41
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.

let image in gr.Image just fit inside the image container
5 participants