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

Add ability to zoom in/out on all images #38538

Merged
merged 3 commits into from Jan 23, 2018
Merged

Add ability to zoom in/out on all images #38538

merged 3 commits into from Jan 23, 2018

Conversation

bschlenk
Copy link
Contributor

Allows smaller images to be scaled up when viewing, either by pinching the trackpad, or holding ctrl while scrolling. Adds image-rendering: pixelated to the css if the image size is below a threshold.

@roblourens
Copy link
Member

roblourens commented Nov 16, 2017

Is there an issue for this?

@bschlenk
Copy link
Contributor Author

Yes, but it was recently closed: #18987

@mjbvz mjbvz requested review from bpasero and mjbvz November 16, 2017 21:07
@bpasero bpasero assigned bpasero and unassigned mjbvz Nov 17, 2017
@bpasero bpasero added this to the On Deck milestone Nov 17, 2017
@bschlenk bschlenk changed the title Add ability to zoom in on small images Add ability to zoom in/out on all images Nov 20, 2017
@bpasero
Copy link
Member

bpasero commented Nov 27, 2017

@bschlenk this is pretty cool now. some thoughts:

  • out of the box an image now looks pixelated even though I did not zoom. just compare a medium sized image how it looks like in stable vs. your changes. any reason to go down that route?
  • currently when you change the zooming and you go from one tab to another, this information is lost once you come back to the image. could we store the zooming info into a cache for this purpose and restore it if possible?
  • I suggest to show the 100% in the status bar always even when not zoomed in initially
  • there is currently no easy way to get back to a 100% zoomed state. is there any other gesture we could add to do this?

@bpasero bpasero modified the milestones: On Deck, November 2017 Nov 27, 2017
@bschlenk
Copy link
Contributor Author

@bpasero

  • out of the box an image now looks pixelated even though I did not zoom. just compare a medium sized image how it looks like in stable vs. your changes. any reason to go down that route?
    • I'm not seeing this behavior. Can you share a screenshot? I have it set to add image-rendering: pixelated if the width of the image is 256px or less. I couldn't find a good heuristic for when to show an image as pixelated, any thoughts on this? It's necessary when viewing pixel art, otherwise it looks really blurry.
  • currently when you change the zooming and you go from one tab to another, this information is lost once you come back to the image. could we store the zooming info into a cache for this purpose and restore it if possible?
    • Ah, I didn't realize it was being reset each time. Is there any way to persist information in a tab while it is still open? I don't know how I feel about caching for this case, since the info would persist until vscode was closed, even if the image was never opened again.
  • I suggest to show the 100% in the status bar always even when not zoomed in initially
    • I initially considered that, but there's a complication. If the image's natural size is larger than the window and it hasn't been zoomed yet, I have it fit within the window. This means that changing the size of vscode will change the size of the image, and thus the % zoom. Does it make sense to add a listener on window resize and update the % zoom for that case?
  • there is currently no easy way to get back to a 100% zoomed state. is there any other gesture we could add to do this?
    • How about double clicking? Or some combination of keyboard + click?

Is there an easy way to grab settings from within resourceViewer.ts? There are some settings I thought would be nice to have:

  • pixelationThreshold: "always" | "never" | number
    • Whether to always pixelate, never, or given a number only pixelate if width or height is less than that number
  • initialZoom: "natural" | "fit" | "natural-fit"
    • natural - initially show images at their natural size
    • contain - initially show images zoomed to fit within the window
    • natural-fit - either show images at their natural size, or fit to the window if they would overflow (current behavior)

@bpasero
Copy link
Member

bpasero commented Nov 30, 2017

@bschlenk

  • I'm not seeing this behavior. Can you share a screenshot? I have it set to add image-rendering: pixelated if the width of the image is 256px or less. I couldn't find a good heuristic for when to show an image as pixelated, any thoughts on this? It's necessary when viewing pixel art, otherwise it looks really blurry.

Yes, it seems t happen for any image smaller 256px, e.g. see this:

image

Compared to (in stable):

image

  • Ah, I didn't realize it was being reset each time. Is there any way to persist information in a tab while it is still open? I don't know how I feel about caching for this case, since the info would persist until vscode was closed, even if the image was never opened again.

No, not at the layer of the resource viewer. I do not think putting it in a cache in there would be an issue.

  • I initially considered that, but there's a complication. If the image's natural size is larger than the window and it hasn't been zoomed yet, I have it fit within the window. This means that changing the size of vscode will change the size of the image, and thus the % zoom. Does it make sense to add a listener on window resize and update the % zoom for that case?

It would be nice to do this within the resource viewer to not break layers. We can enrich the resource viewers method to return an object that we can call layout on and then just call layout whenever the editors size change where the resource viewer is being used in (here).

  • How about double clicking? Or some combination of keyboard + click?

Or right click maybe? Is there an example of how other tools are doing it?

Is there an easy way to grab settings from within resourceViewer.ts? There are some settings I thought would be nice to have:

  • pixelationThreshold: "always" | "never" | number
    • Whether to always pixelate, never, or given a number only pixelate if width or height is less than that number
  • initialZoom: "natural" | "fit" | "natural-fit"
    • natural - initially show images at their natural size
    • contain - initially show images zoomed to fit within the window
    • natural-fit - either show images at their natural size, or fit to the window if they would overflow (current behavior)

Not sure I would want settings for this. After all we are not an editor for images, so we should have a good out of the box experience.

@bschlenk
Copy link
Contributor Author

bschlenk commented Dec 1, 2017

I'm not sure why your screenshot would look pixelated, according to MDN image-rendering has no effect on non-scaled images. I could reduce the threshold to 56px, but I have some images larger than that which should display pixelated. Is it easy to add a right click menu? A pixelated/non-pixelated option could be added there.

Photoshop uses cmd + 1 to zoom 100%, but that's already used to change editor windows. Gimp uses shift + ctrl + E, but that feels convoluted. This seems like another candidate for a right click menu. But if that's not an option, then I think right click makes sense as the return to 100% function.

@bpasero
Copy link
Member

bpasero commented Dec 1, 2017

If it helps, I have an external monitor connected to my MacBook Pro with a resolution of 2560x1440. Why do we need to set this property at all?

@bpasero bpasero modified the milestones: November 2017, On Deck Dec 1, 2017
@bschlenk
Copy link
Contributor Author

bschlenk commented Dec 3, 2017

For viewing pixel art, this is why it matters:
screen shot 2017-12-02 at 5 00 47 pm

The left one doesn't have image-rendering: pixelated, the right one does. I think it would be a miss to not at least include an option for pixelated rendering, if it isn't automatically set based on certain image properties.

@bpasero
Copy link
Member

bpasero commented Dec 3, 2017

Yeah I think it makes sense once you are zooming in at least.

@bpasero bpasero removed this from the On Deck milestone Dec 7, 2017
Images are now always centered in the window. They initially start at
their native size, unless they would be larger than the window, in which
case they are contained within the window. Clicking increases the
zoom, and alt+click decreases it. Pinch to zoom and ctrl+scroll are also
supported.
ResourceViewer now holds a cache of image scales so they stay the same
while flipping between editor tabs. Right clicking now returns the image
to its original scale. Pixelation only triggers for images 64x64 or
smaller, and only after the first zoom. Editor risizing is handled
thorugh the layout call to the binary editor, passed down to the
resource viewer.
@bschlenk
Copy link
Contributor Author

@bpasero Any updates on this getting merged?

@mjbvz mjbvz added this to the January 2018 milestone Jan 19, 2018
@bpasero
Copy link
Member

bpasero commented Jan 19, 2018

@mjbvz you seem to want to own this, so feel free. make sure to add a test plan item and add me as tester.

@bpasero bpasero removed their request for review January 19, 2018 08:36
@bpasero bpasero removed their assignment Jan 19, 2018
@mjbvz mjbvz merged commit 9081507 into microsoft:master Jan 23, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants