fix: preview thumbnails don't scale as large as they could

[Computerdores]

Summary

Due to a hardcoded maximum size for preview thumbnails, preview thumbnails don't scale to fit all the space available to the PreviewThumb Component. Since a limit is needed to avoid the performance hit that massive images would incur, I have change the limit to be the screen size since the component shouldn't be bigger than that anyways.

Tasks Completed

• Platforms Tested:
  • Windows x86
  • Windows ARM
  • macOS x86
  • macOS ARM
  • Linux x86
  • Linux ARM
• Tested For:
  • Basic functionality
  • PyInstaller executable

[Computerdores]

Before:

After

[CyanVoxel]

Nice fix! Although using the screen size can come with a decent performance hit depending on the screen resolution and underlying file type. For example, SVG files now take about a full second to render in the preview panel for me instead of showing instantly since they're now always rendering at 5K.

One alternative that gets around this problem could be to leverage the self.__img_button_size variable already in the class:

def __render_thumb(self, filepath: Path) -> None:
        self.__thumb_renderer.render(
            time.time(),
            filepath,
            self.__img_button_size,
            self.devicePixelRatio(),
            update_on_ratio_change=True,
        )

This has the upside of only rendering what's needed for the preview panel's current size, but comes with the drawback of not automatically filling in the entire space available if the preview panel is made larger after rendering until re-rendering the preview. Perhaps some sort of threshold check inside the resizeEvent to trigger a re-render could fix that though?

[Computerdores]

Nice fix! Although using the screen size can come with a decent performance hit depending on the screen resolution and underlying file type. For example, SVG files now take about a full second to render in the preview panel for me instead of showing instantly since they're now always rendering at 5K.

One alternative that gets around this problem could be to leverage the self.__img_button_size variable already in the class:

def __render_thumb(self, filepath: Path) -> None:
        self.__thumb_renderer.render(
            time.time(),
            filepath,
            self.__img_button_size,
            self.devicePixelRatio(),
            update_on_ratio_change=True,
        )

This has the upside of only rendering what's needed for the preview panel's current size, but comes with the drawback of not automatically filling in the entire space available if the preview panel is made larger after rendering until re-rendering the preview. Perhaps some sort of threshold check inside the resizeEvent to trigger a re-render could fix that though?

Yeah... this pretty much sums up most of the thoughts I have had on this. Main reason why I did it as it is, is that this was the path of least resistance. Also, any other approach requires rerendering on resize and I am not quite sure how to do that well. Because you either rerender on every resize event (horrible performance) or you try to guess how big the user will make it and only resize when it gets above that (very guessy and kind of feels icky to me, not sure that's a good enough argument though 😅)

[CyanVoxel]

Yeah... this pretty much sums up most of the thoughts I have had on this. Main reason why I did it as it is, is that this was the path of least resistance. Also, any other approach requires rerendering on resize and I am not quite sure how to do that well. Because you either rerender on every resize event (horrible performance) or you try to guess how big the user will make it and only resize when it gets above that (very guessy and kind of feels icky to me, not sure that's a good enough argument though 😅)

The real path of least resistance would be to hardcode it at 512x512 and not touch it for another year 😉

I think I actually managed to get a solution going that manages to stay performant for low-res previews while allowing them to dynamically scale and only re-rendering when needed:

@override
    def resizeEvent(self, event: QResizeEvent) -> None:
        self.__update_image_size((self.size().width(), self.size().height()))

        if self.__filepath is not None and self.__rendered_res < self.__img_button_size:
            self.__render_thumb(self.__filepath)

        return super().resizeEvent(event)

Here I'm keeping track of a __rendered_res variable inside the __render_thumb() method and then comparing that value against the current size during the resizeEvent. On my display it only re-renders up to one additional time when going from a small preview to a massive one.

def __render_thumb(self, filepath: Path) -> None:
        self.__filepath = filepath
        self.__thumb_renderer.render(
            time.time(),
            filepath,
            self.__img_button_size,
            self.devicePixelRatio(),
            update_on_ratio_change=True,
        )
        self.__rendered_res = (
            math.ceil(self.__img_button_size[0] * self.devicePixelRatio()),
            math.ceil(self.__img_button_size[1] * self.devicePixelRatio()),
        )

If you want I can commit this, or you could experiment with your own implementation as well

[Computerdores]

On my display it only re-renders up to one additional time when going from a small preview to a massive one.

I am assuming you pressed the full screen button of the window? Because for me when dragging the slider of the splitter, for example, it issued multiple resize events (in testing from earlier, not at my pc atm so can't test)

[CyanVoxel]

On my display it only re-renders up to one additional time when going from a small preview to a massive one.

I am assuming you pressed the full screen button of the window? Because for me when dragging the slider of the splitter, for example, it issued multiple resize events (in testing from earlier, not at my pc atm so can't test)

I had to use about ¾ of my monitor height for a re-render to fire, I didn't check as much horizontally since it covered up more and more of my console...

I should also clarify, the resize events are always happening whenever the splitter moves or those sizes change - but the check against the current size vs the last rendered size is what filters out the need to re-render for me. I should do some tests on my normal DPI monitor to see if it still works as intended there though

[CyanVoxel]

Aha, it was a quirk of the high DPI monitor that let this work as-is, and trying it on a 100% scaled monitor showed the same issues. However, this also illuminated a solution to make it always work how it did for me - Using a 2x multiplier as the cutoff for re-rendering:

def __render_thumb(self, filepath: Path) -> None:
        self.__filepath = filepath
        self.__rendered_res = (
            math.ceil(self.__img_button_size[0] * 2),
            math.ceil(self.__img_button_size[1] * 2),
        )

        self.__thumb_renderer.render(
            time.time(),
            filepath,
            self.__rendered_res,
            self.devicePixelRatio(),
            update_on_ratio_change=True,
        )

Now it consistently only re-renders for me when the preview increases in size by a large amount and works across different resolutions and DPIs.

[Computerdores]

what are you setting self.__filepath for? Couldn't you instead check whether self.__rendered_res is None?
nvm

[Computerdores]

I implemented what you suggested, only change is that I moved the factor to a constant so it can be tweaked more easily

[CyanVoxel]

I implemented what you suggested, only change is that I moved the factor to a constant so it can be tweaked more easily

Looks good! Probably just want to have self.__filepath and self.__rendered_res declared and/or initialized at the class level?

[Computerdores]

I implemented what you suggested, only change is that I moved the factor to a constant so it can be tweaked more easily

Looks good! Probably just want to have self.__filepath and self.__rendered_res declared and/or initialized at the class level?

Damn I knew I missed something, fixed now

[CyanVoxel]

Awesome work on this, thank you so much!