Skip to content

Conversation

@jcupitt
Copy link
Member

@jcupitt jcupitt commented Jul 26, 2025

It's almost there, it fails with:

1313.9 ld.lld: error: undefined symbol: ___chkstk_ms

Perhaps because it's building (or linking?) with g++ and not clang?

I pointed vips-all.mk at 8.18.0-test1 just for testing, using --nightly would probably be better.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

Of course there's an upstream libraw.mk, I should have checked.

It still fails to link though. I tried 0.21.4 and 0.21.1 (the upstream version of libraw).

I'll try without jasper.

@jcupitt jcupitt marked this pull request as draft July 27, 2025 10:23
The subdirectory inside the tarball lacks the test release suffix.
@kleisauke
Copy link
Member

Ah, the ___chkstk_ms linker error is a known issue with libtool based projects that contain C++ code.
https://github.com/mstorsjo/llvm-mingw/blob/20250709/README.md?plain=1#L128-L147

Commit fbfe0f0 should fix that.

@kleisauke
Copy link
Member

Seems to work!

Screenshot

@jcupitt jcupitt marked this pull request as ready for review July 27, 2025 13:54
@jcupitt jcupitt marked this pull request as draft July 27, 2025 13:54
@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

So cool!

Did you try viewing a zone plate? Are the tile borders fixed?

I'll build here too.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

... I thought of another question, is there any way to get openmp under mingw? Though I don't know how much libraw benefits, if at all.

@kleisauke
Copy link
Member

Did you try viewing a zone plate? Are the tile borders fixed?

The tile borders were no longer visible on the zone plate image the last time I checked.

I thought of another question, is there any way to get openmp under mingw?

The last time I looked into OpenMP, it didn't seem worth the effort. It was also a bit tricky to cross-compile since a MASM-compatible assembler was required.

In the 'web' group dependencies, OpenMP is only used in Pixman for blue noise generation. In the 'all' group, aside from LibRaw, only FFTW and IM would benefit from it.

@kleisauke
Copy link
Member

I can confirm that the tile borders are no longer visible with vipsdisp v4.1.0.

Screenshot

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

Great! I built here too and uploaded the binaries to the 4.1.0 release on github. Thank you very much for doing all this work, Kleis!

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

Oh drat, I see borders here:

image

Maybe it's just my win10 VM? It's running with software rendering, maybe that makes a difference?

@kleisauke
Copy link
Member

Which GSK renderer are you using? You can check that with Control + Shift + I and then clicking the "Global"-tab.

I was able to reproduce this by setting the GSK_RENDERER=cairo env variable. It looks like MR https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/8494 was only done for the Vulkan/GL renderer. We likely need to patch gsk_texture_scale_node_draw() to support this in the fallback Cairo renderer.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

Yes, cairo since I'm on software render. I didn't realise the gtk patch was hardware render only, that's good to know.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

A friend is seeing tile borders with the GL GSK renderer :( I'll try on another PC here.

@kleisauke
Copy link
Member

A friend is seeing tile borders with the GL GSK renderer

That's odd. I'm not entirely sure if the renderer shown in the GTK Inspector reflects the actual backend in use, it might(?) silently fallback to Cairo.

You can debug this by setting the GDK_DEBUG=opengl env variable. Although vipsdisp is built with -Wl,-subsystem,windows, you can still get debug output by setting G_WIN32_ATTACH_CONSOLE=stdout,stderr.

On my Windows machine using cmd.exe, I see:

$ chcp 65001
Active code page: 65001
$ set G_WIN32_ATTACH_CONSOLE=stdout,stderr
$ set GDK_DEBUG=opengl
$ vipsdisp
requesting pixel format...
requested and set pixel format: 18
Creating core WGL context (version:3.3, debug:no, forward:no)
Created WGL context[0000000000010001], pixel_format=18
OpenGL version: 4.6 (core)
GLSL version: 4.60 NVIDIA
Max texture size: 32768
Enabled features (use GDK_GL_DISABLE env var to disable):
    debug: ✓
    base-instance: ✓
    buffer-storage: ✓
WGL API version 4.6 found
 - Vendor: NVIDIA Corporation
 - Renderer: NVIDIA GeForce GTX 1070/PCIe/SSE2
 - Quirks / disallow swap exchange: enabled
 - Checked extensions:
        * WGL_ARB_pixel_format: yes
        * WGL_ARB_create_context: yes
        * WGL_EXT_swap_control: yes
        * WGL_OML_sync_control: no
        * GL_WIN_swap_hint: yes
Using OpenGL backend Windows WGL

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

I tried on another PC here (an old Dell with a nv 1060) and it's working fine. It also shows GL as the GSK backend.

I'll ask my friend to try with GDK_DEBUG.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

He's seeing this:

image

It looks the same as the one that works :(

@kleisauke
Copy link
Member

Does the image in question have transparency? I just noticed that the backdrop tiles are added using gtk_snapshot_append_texture(), but I only cherry-picked the commits related to texture scale nodes (i.e. the gtk_snapshot_append_scaled_texture() API).

Commit a43a0b6 ensures that textures added using gtk_snapshot_append_texture() are also snapped to the pixel grid.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

I think he was viewing RGB, not RGBA.

But I changed the tilecache -- it's always RGBA now, and always 256 * 256. That's so you can be showing an RGB image, then flip to an RGBA image and it won't need to throw the whole tree away, so you can prevent the display flashing black for a moment.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 27, 2025

Could it be related to high DPI? I think he saw the lines on a 4k display with 200% scaling.

@kleisauke
Copy link
Member

I was able to reproduce this issue on both GL and Cairo renderers by setting the GDK_SCALE=2 env variable. I'll try updating to GTK 4.19.2 to see if that resolves it, although testing will be more complicated since support for that variable was removed in https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/8507.

@kleisauke
Copy link
Member

Ah, I was able to set the scaling to 200% through Windows' advanced scaling settings and can confirm it causes thin white lines.

Commit f35126c updates GTK to 4.19.2, but unfortunately, that didn't fix it.

@kleisauke
Copy link
Member

I also noticed this interesting merge request: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/8503.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 29, 2025

How frustrating!

Perhaps we should add a "snap rectangle to nearest hardware pixel boundary" function to vipsdisp instead? It should be simpler than patching gtk.

I think I can see what to do, I'll try a quick PR.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 29, 2025

I had a quick hack, what do you think?

libvips/vipsdisp#51

It seems to work OK on linux, including fractional desktop scaling, but I've not tested elsewhere yet.

@kleisauke
Copy link
Member

Since I could also reproduce this issue on Fedora, perhaps we should provide early feedback on the upstream merge request?

Here's a standalone reproducer:

Details
$ gtk4-rendernode-tool show ~/Downloads/zone-plate.node

zone-plate.node (saved using the recorder in GTK inspector):
zone-plate.zip

With 100% scaling:
Screenshot From 2025-07-30 13-05-55

With 125% scaling:
Screenshot From 2025-07-30 13-09-07

@jcupitt
Copy link
Member Author

jcupitt commented Jul 30, 2025

That's a good idea. I'm sure they'd also be interested to know that that someone else is looking at this issue.

Does your example also fail with 200% scaling? Int and float scaling issues are usually considered separately I think.

Another related issue is high-DPI support in general. I've not been able to render a one-pixel checkerboard image like this:

checkerboard

As a 1 image pixel == 1 hardware display pixel on a 200% scaled desktop on Ubuntu 25.04. I think (I think!!) something in gtk is filtering it in error, even if I select NN as the node filter. I usually get a solid black or solid white image by mistake. Perhaps there's something else I need to do to enable high-DPI textures?

No high DPI support in texture node rendering means you have to round tiles to 2 pixel boundaries, and that in turn causes visible rippling, or a "jelly" effect as you scroll a tiled image.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 30, 2025

To reproduce the high-DPI problem, build this PR libvips/vipsdisp#51 without gtk snapping, then load the checkerboard image, select 1:1, and scroll to the top-left of the image. I see:

tilecache_snapshot: 0x646201771ff0 scale = 1, x = 0, y = 0
  paint x = 0, y = 0, width = 487, height = 114
  viewport image0 coordinates x = 0, y = 0, width = 974, height = 228

So 114 high paint rect (gtk coordinates) fetches 228 lines of pixels from the image, ie. the 256x256 pixel tiles are drawn at 128x128 in gtk coordinates. You can use "d" to see the tiles are nice and small now. But the image is wrong:

image

EVen with NN the 0/255 pixels have become a flat 127.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 30, 2025

Ooof I changed my mind, sorry, this branch now has the high-DPI experiment https://github.com/jcupitt/vipsdisp/tree/add-high-dpi

https://github.com/jcupitt/vipsdisp/tree/add-tile-snap now just has tile snapping in vipsdisp and no attempt at high-DPI.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 30, 2025

... I merged the tilesnapping code to vipsdisp master. It has some issues, but the white lines are gone for everything except the checkerboard image on linux and win, even with fractional desktop scaling.

@kleisauke
Copy link
Member

Great! I just added comment https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/8494#note_2507714 to the upstream MR.

I'll look into that high-DPI issue later today.

@kleisauke
Copy link
Member

... according to upstream, we should probably be doing this first:

/* Snap the textures to a physical pixel.
 */
gtk_snapshot_scale(snapshot, pixel_size, pixel_size);

(I also noticed that this was done in MR https://gitlab.gnome.org/GNOME/papers/-/merge_requests/505)

I had a very quick go with commit kleisauke/vipsdisp@1cd9844, which seems to work, but some further work is needed to get the positioning right.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 30, 2025

I hadn't seen that API! Yes, that seems to fix high-DPI! It fixes the jelly effect too.

I'll try a PR to fix the sizing and coordinate issues.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 31, 2025

This seems to add high-dpi correctly:

libvips/vipsdisp#52

We now have no white lines, even with fractional display scaling, and 1:1 image pixel == hardware display pixel! No jelly effect on scroll either.

(tested on ubuntu 25.04 with various desktop display densities)

@kleisauke
Copy link
Member

Great! I'll test this on Windows later today.

@jcupitt
Copy link
Member Author

jcupitt commented Jul 31, 2025

Ah fantastic! Then I think we're done here.

@kleisauke kleisauke marked this pull request as ready for review August 1, 2025 10:17
Copy link
Member

@kleisauke kleisauke left a comment

Choose a reason for hiding this comment

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

Thank you! I just created the 8.17 branch, which will serve as the base for future v8.17.x updates.

@kleisauke kleisauke merged commit 89d7736 into master Aug 1, 2025
@kleisauke
Copy link
Member

https://github.com/libvips/build-win64-mxe/releases/tag/v8.18.0-test1 🎉

I also commented on the upstream merge request, noting that this was indeed the issue we ran into.

@jcupitt jcupitt deleted the add-libraw branch August 1, 2025 10:46
@kleisauke kleisauke mentioned this pull request Aug 2, 2025
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.

3 participants