YavorGIvanov / sam.cpp

MIT License
1.24k stars 52 forks source link

automatically downscale image when exceeds screen #11

Closed tizee closed 10 months ago

tizee commented 10 months ago

I found that if the image resolution is really large then the SDL window probably exceed the size of the screen. So this PR aims to resolve this issue.

I wonder if it's okay to add a scale parameter to scale down the window size by downsizing the image displayed in the window. After testing the workaround on my machine, I think it has resolved the window size issue when the image resolution is larger than the screen's.

The resizing algorithm I used is simple, which is the nearest neighbor interpolation.

YavorGIvanov commented 10 months ago

It seems you are still creating the old texture, which I feel is not needed if we go with your solution to the problem. However, I would prefer to not solve the problem by introducing a command line argument, but doing it automatically, when we detect that the screen is smaller than the image.

tizee commented 10 months ago

I understand your idea. Does it mean:

  1. When image is smaller than the screen, then we do nothing.
  2. If the image size is larger than the screen, then we need to downscale the image automatically.

Then there is no need to introduce a parameter. This would be more robust since there is no need to try to figure out the correct factor for downscale.

I'm willing to implement this if you're not going to do that right now.

YavorGIvanov commented 10 months ago

Yes. You can do that. For getting the screen size I guess you can use:

    SDL_DisplayMode display_mode = {};
    SDL_GetCurrentDisplayMode(0, &display_mode);
    const int screen_width = display_mode.w;
    const int screen_height = display_mode.h;

or something similar.

tizee commented 10 months ago

@YavorGIvanov I have to note that the reason why the resize algorithm for sam_image_u8 uses a uint_t scale factor instead of a float factor is that when we pass in a float scale factor then it causes the image twisted. What's more, we are modifying asam_image_u8 type instead of a sam_image_f32 type. Here I also just want to make it consistent in type.

YavorGIvanov commented 10 months ago

I added some bug fixes and suggestions in a separate PR (Couldn't figure out how to better show them) #16 Check them out and tell me what do you think ? You can either integrate them here if you agree with them or we can merge the other PR.

tizee commented 10 months ago

The PR #16 looks good and I've tested it on machine. I think we can merge the other PR directly.

YavorGIvanov commented 10 months ago

Rebased the other PR and merged it. Thank you for the contribution. 🚀