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

Resizable Image Previews #2167

Merged
merged 11 commits into from
Sep 10, 2023
Merged

Resizable Image Previews #2167

merged 11 commits into from
Sep 10, 2023

Conversation

joeyballentine
Copy link
Member

@joeyballentine joeyballentine commented Aug 30, 2023

Self explanatory. Closes #782

Decisions I had to make:

  • It now only encodes the largest image preview, up to 2048x resolution. Maybe in the future we could make this value customizable for performance or something.
  • Outputs can now save their size in nodeState

image
image
image

@RunDevelopment
Copy link
Member

It now only encodes the largest image preview, up to 2048x resolution.

This is a big issue, because it will noticeably degrade performance on non-high-end devices. Why did you remove that anyway?

@joeyballentine
Copy link
Member Author

Why did you remove that anyway?

Because the size of the image itself is now more than just a display thing. If you make the node full res and the UI switches it to the smaller image, it'll shrink the image being displayed.

I'm not sure how to get around that besides always stretching the image to the bounds of the node, which I'm not sure could be made to retain aspect ratio. I'm open to suggestions

@joeyballentine
Copy link
Member Author

I did figure out how to get around that btw. It works pretty well now

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Code looks good. Just a few questions.

@RunDevelopment
Copy link
Member

Found some bugs:

  1. Resizing when snap to grid is enabled: With 16px grid size, you cannot resize the image preview back to its smallest size.
  2. Output sizes are not saved.

@joeyballentine
Copy link
Member Author

Resizing when snap to grid is enabled: With 16px grid size, you cannot resize the image preview back to its smallest size.

I don't think this is something I can really fix. There's actually a bunch of issues with snap to grid all around chaiNNer, like iterator child nodes popping out of the iterator container. IMO this is a non-issue and the real issue is we need to better support snap to grid somehow

@joeyballentine joeyballentine merged commit 2efd37e into main Sep 10, 2023
4 checks passed
@joeyballentine joeyballentine deleted the resizable-img branch September 10, 2023 20:40
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.

Custom size node (for View Image)
2 participants