Skip to content

Conversation

@mdesmedt
Copy link
Contributor

This PR adds a new ScalingMode enum and a set_scaling_mode function. The scaling mode defaults to ScalingMode::PixelPerfect which preserves the current integer upscaling code.

ScalingMode::LinearPreserveAspect is a simple bilinear upscale which preserves the input buffer aspect ratio.

ScalingMode::LinearStretch is a simple bilinear upscale which does not preserve aspect ratio.

ScalingMode::PixelPerfectHybrid is the most interesting one. It's basically a single pass "integer+linear" upscaling approaching using custom bilinear filtering weights. It preserves the input buffer aspect ratio. It was inspired by https://csantosbh.wordpress.com/2014/01/25/manual-texture-filtering-for-pixelated-games-in-webgl/

The names for the enum fields were just a quick guess on my part. "PixelPerfectHybrid" is maybe not the best naming choice as it's not 100% pixel perfect.

@parasyte
Copy link
Owner

parasyte commented Jul 11, 2025

Hi! No real concerns about the code or code quality, but I am going to be critical of some of the features in this.

Simple bilinear filtering is (IMHO) not on the table. It looks terrible when the scaling factor is too large (it's hard to say what "too large" is quantitatively). The non-aspect-preserving stretch also goes against the grain here. I'm not really interested in supporting these use cases, even if they are trivial, due to output quality concerns.

The last mode (hybrid) is interesting, but reminds me of an antialiasing algorithm. I can't quite see the use case for this one (but see my notes below that compares it to an alternative). The pixel grid in pixels is always axis-aligned. It isn't like a game where you might want to rotate a sprite (in 2D or 3D) after it has been scaled. Any rotation or shearing will only happen pre-scaling, and they would be software-rasterized (rotsprite or whatever, totally up to the user to do any of that).

However, it does remind me of something that I implemented in the precursor to pixels: https://blog.kodewerx.org/2017/09/tiny-mcu-3d-renderer-part-6-camera.html This is specifically for supporting non-square pixel aspect ratios. E.g. NES has an approximately 8:7 pixel aspect ratio, making pixel perfect integer scaling impractical (the scaling factor along the X axis must be divisible by 8 and the scaling factor along the Y axis must be divisible by 7)! The trick is instead to scale with nearest using the preferred integer factor, then scale only horizontally with a linear filter by a factor of x/y. The result is minimal blurring for a non-integer scale.

Taking the same idea one step further, rather than scaling only horizontally to compensate for the pixel aspect ratio, the second pass could scale in both directions to fill the screen. This will also result in minimal blurring. (Except in degenerate cases where the nearest pass uses a 1x scale factor.) The two-pass scaling also generalizes to a single-pass fragment shader, which is very close to the hybrid shader (the main difference I can see is that the alpha parameter is chosen based on the ratio between the source and destination texel sizes, rather than fixed).

I started a draft PR long ago in an effort to implement pixel aspect ratio support using the former method described in my blog post: #151. This thread contains more context, and it can be used as a point of comparison (but maybe not much more than that: it's based on a very outdated revision).

Anyway, I would love to support a "fill the screen, I don't care if it's exactly pixel perfect" scaling mode, but I absolutely want to preserve crispness in the output as much as possible. At least in the main crate, which strives to do the right thing with low effort on the user's part.

@mdesmedt
Copy link
Contributor Author

Well the use case for the "hybrid" scaling is that it's comparable to an integer upscale followed by a linear upscale. I am not aware of a better solution to fit a pixel buffer to the window, in a non-integer ratio, while preserving the sharp borders between input pixels. Implementing it as a single pass is more efficient and avoids having to allocate a temporary texture, and it avoids having to complicate the current code.

As a pixels user for a software rasterizer I have no upscaling preference and would have been content with a straightforward linear upscale for my usecase. I was just looking for a solution to fit to the window. But I respect if you want to only support certain approaches.

@mdesmedt
Copy link
Contributor Author

I could just call the Hybrid mode ScaleMode::Fill or something and remove the two simple linear modes. The hybrid shader looks quite good to me even for 10x scaling. What do you think?

@parasyte
Copy link
Owner

parasyte commented Jul 11, 2025

I think it's a good start! Could you also enable the fill scaling mode on at least one of the existing examples?

I tried it on invaders, and it appears to be blurry on macOS. The default scaling factor for this example is 3x logical pixels. Apple displays have a native 2x DPI scaling factor, so the surface is scaled up 6x relative to the source image. I haven't tried it on a display with 1x DPI scaling factor yet.

Here are some comparison screenshots, cropped from the invaders example. The first table shows the actual output size (6x) with nearest filter on the left, and hybrid on the right.

Nearest Hybrid
tank-nearest tank-hybrid
invaders-nearest invaders-hybrid

The blur is not completely obvious, but it can be clearly seen when zooming in on the detail.

These are scaled up by another 3x in GIMP with no interpolation (18x total scaling factor relative to the source image):

tank-nearest-3x tank-hybrid-3x
invaders-nearest-3x invaders-hybrid-3x

The worst case I would expect for antialiasing would mix colors in a maximum of 1 row and column in the image with no adjacent mixing, as I discuss in my blog post. These images are showing two adjacent rows and columns of antialiasing.

But these are integer scaling factors. There should be no mixing at all.

@mdesmedt
Copy link
Contributor Author

mdesmedt commented Jul 12, 2025

You're right, that was a bug. I believe I have fixed it. At least from what I can see integer ratios scale cleanly now, while preserving one pixel of interpolation for non-integer scales.

I could not reproduce the two pixels of interpolation by the way. Perhaps it is fixed now (as this change in the shader makes more sense to me) but please check on your end.

@parasyte
Copy link
Owner

Thank you! It is looking really solid, now. I think it is behaving almost exactly as I expected.

There will probably be more lints to resolve.

@parasyte
Copy link
Owner

For the too many arguments lint, I think it makes sense to create intermediate structs to replace multiple args. Most of the WGPU types can be combined into one struct, for instance.

The other option is ignoring the lint (#[allow(clippy::too_many_arguments)]) on that one function, but it should be used sparingly. The attribute is currently used on (at least) create_backing_texture(). Many of those args just pass through to ScalingRenderer::new() anyway. An intermediate struct might help.

@mdesmedt
Copy link
Contributor Author

I'm just going to add #[allow(clippy::too_many_arguments)] for now. I can see why you might want to change it but I feel that's beyond the scope of this PR.

@parasyte
Copy link
Owner

Alright, I'll merge this and maybe follow up with some additional PRs later.

Thank you!

@parasyte parasyte merged commit 8c67065 into parasyte:main Jul 23, 2025
9 checks passed
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.

2 participants