Xv5 HiDPI, C23, fixes

[william8000]

This patch series adds support from other xv forks plus adds HiDPI support, updates to C23, clears all compile warnings from gcc-14 and clang-18, and fixes some bugs.

[tanabi]

Hooo boy this is a monster. I've started to review it, but its the end of a long day for me so I'm probably not going to get too far with it :)

[mdadams]

Wow! There is a lot of work there. I did a quick pass through all of the changes. As best I can tell, it appears that the code changes are doing what is described in the PR and the associated commit log messages. There is a lot of cleanup that has been done to make the code less archaic looking, which is a good thing.

@william8000 Thank you for this very substantial work.

@tanabi After you have had a chance to look at the code changes in more detail, please let me know what you think. Barring any unforeseen circumstances (such as any serious problems that you might find), my current inclination would be to merge the PR, although I'll have to fix the conflicts (since unfortunately there are some).

@thargol Are you okay with any changes to the WEBP support in this PR?

[william8000]

@mdadams Sorry about the conflicts. If you want, I can fix that. I should have rebased before applying the last set of commits from your master that came after I made the fork.

The commits referencing patches came from https://github.com/DavidGriffith/xv/commits/master/ I gave credit to the contributions in my commits where I could find them. Everything else is mine, and I grant the xv project whatever authorization is necessary.

[william8000]

The webp changes add a save dialog that I integrated based on a commit in David Griffith's fork.

[william8000]

I did a rebase to fix the merge conflicts. It should be the same code as before, but git is happier.

I tested all of the image formats in the data/images directory plus the PIC, PIC2, MAG, and MKI image types.

The VMS changes are untested. I used to do development with vax11c on VMS on a VAX-11/750 in the 1980's, so I could probably do a VMS port if I had access to a VMS server.

[mdadams]

@william8000 Thanks for the rebase. It is better not to have any conflicts.

[mdadams]

Hmm... Does anyone still use xv on VMS? I would hope that no one is still using VMS, but some OSes never die. :smiley:

[thargol]

I'm literally on my way to the airport to go away for a week. Superficially it looks fine. I can take a proper look when I get back.

[william8000]

I suspect that no one runs xv on VMS. Compaq bought DEC. HP bought Compaq. HP sold VMS to VSI, which is working on an x86-64 port. https://vmssoftware.com/

[tanabi]

@mdadams I'll try to get a final yea / nea vote this evening, I got about halfway through the PR yesterday :) I wish I could test this more, I don't have a hi dpi display though. I generally agree that VAX support probably isn't necessary. If someone is still using xv on VAX (which wouldn't surprise me TBH) then they are probably pinned to an earlier version anyway. I'm sure not all of our libraries support VAX.

[william8000]

@tanabi The changes add command line options so you can test hidpi with 'xv -hidpi' or 'xv -dpimult 2'. It will make everything twice as large on a normal display. I updated a lot of dialogs but there might be dialogs that I missed. The only thing that you need a hidpi display to test is the automatic detection of the display. For both normal and hidpi, there isn't always a good selection of fonts. It is more visible with hidpi. I took what worked on my Fedora 40 laptop, but the available fonts could depend on the OS and distribution. I was thinking about adding support for Xft, which can build nice looking fonts at any size. There is no rush to decide on the pull request, and I can make fixes if you have suggestions.

[william8000]

@tanabi Thanks for reviewing it.

I removed the -w1 version suffix when I did the rebase. The commit is https://github.com/jasper-software/xv/pull/23/commits/6d32b30a62c6b0e3d748c97922d44b741cb9f039 The comparison before and after the rebase is https://github.com/jasper-software/xv/compare/ad90d7f2a4daac3a5896ca2d27ec4df6e572b534..45b8d8be306327e8d3e02686d8b099fc3d292ab3 That is the only diff from the rebase. Years ago, I tried to make a current xv, and I started with a clone of David Griffith's fork. At the time, it seemed to be the most recent, and I didn't find any xv fork that was maintained. I added the "-w1" suffix so I could tell whether an executable had my patches. I found the jasper-software fork by chance, and when I saw that it was maintained, I combined everything in my clone of David Griffith's fork from the point where it diverged from the jasper-software fork.

The FIXME comment in xvvd.c comes from the last patch in the commit https://github.com/DavidGriffith/xv/commit/c59166e7fa60630e7b4b9ba22f7fe93fea160d2f xvvd.c allows extracting images from archives. It is protected by #ifdef AUTO_EXPAND , which is disabled in config.h, so it never runs. I can take a look at it. In my own code, I usually wrap filenames in double quotes and add a backslash before special characters.

I noticed that my dpiMult got messy. I didn't try to combine numbers like changing 1 + 3 + 2 into 6*dpiMult because the numbers help check the calculations since each number is a different margin or border. Probably the C compiler will optimize it, but even if the C compiler doesn't optimize it, the cost of calculating the dialog is almost nothing compared to the cost of drawing it. I thought about making high-level drawing functions that would multiply their arguments by dpiMult internally, but the font widths and heights are already scaled. Maybe C++ typing would simplify keeping track of scaled vs unscaled values. I think that the deeper issue is that xv draws the dialogs with low-level X functions instead of using a higher level package like gtk or qt or even Xaw3d, but I think that it isn't worth it, considering that Linux already has a number of image viewers.

[mdadams]

@tanabi, thank you for your review of the code changes. It is my understanding that @thargol is planning to review the WEBP changes when they get back from their travels in a week. Based on your review (and @william8000's comments), I would suggest that, as long as @thargol does not raise any significant concerns in their review of the code changes (in about one week's time), we proceed with the merge. Are you okay with this plan?

[william8000]

I'm looking at the FIXME in xvvd.c. xvvd must have been broken for a while because when someone converted mktemp() to mkstemp(), they should have used mkdtemp() in xvvd, because mkstemp() creates a file instead of a directory, which prevents xvvd from creating subdirectories. I added quoting that works like bash using outer single quotes and then converting embedded single quotes to single quote, backslash, single quote, single quote. Both compressed files and xvvd now work for me with file names with single quotes, double quotes, backslashes, and other special characters. I also added support for xz. xvvd supports tar but not compressed tar archives. That probably isn't worth fixing. The last thing left with xvvd is updating the deprecated sigmask/sigblock for the current sigprocmask/sigaction. I'll add it to the pull request in a day or two.

[tanabi]

@mdadams Sounds good to me!

@william8000 I agree with you regarding the dpiMulti ... There are relatively few people still working on this software and we're unlikely to acquire new users in the future. Pretty sure the people that use xv are people who have used it since the 90's :) So its tolerable to do these multiplications everywhere.

If we were ever going to port this to C++, I'd put time into designing that better, but ... I don't see that ever happening :D xv's great, I love seeing it live on, and I use it almost every day. But I'm not interested enough to do a major re-write of it, personally.

[tanabi]

I'm looking at the FIXME in xvvd.c. xvvd must have been broken for a while because when someone converted mktemp() to mkstemp(), they should have used mkdtemp() in xvvd, because mkstemp() creates a file instead of a directory, which prevents xvvd from creating subdirectories. I added quoting that works like bash using outer single quotes and then converting embedded single quotes to single quote, backslash, single quote, single quote. Both compressed files and xvvd now work for me with file names with single quotes, double quotes, backslashes, and other special characters. I also added support for xz. xvvd supports tar but not compressed tar archives. That probably isn't worth fixing. The last thing left with xvvd is updating the deprecated sigmask/sigblock for the current sigprocmask/sigaction. I'll add it to the pull request in a day or two.

That's cool. Yeah, I wouldn't bother with dealing with compressed tars. I've used xv since the 90's as noted above and I'm not sure I have ever used its compressed file feature. I'm sure if it was something people used regularly, it would have either been fixed or complained about by now ... :)

Thanks for the attention on this!

[william8000]

@tanabi I fixed opening compressed tar archives. The main problem was that blocking SIGCHLD breaks the system() call used for uncompression. Also, bzip2 and xz compressed tars work now (in addition to gzipped tars), and compressed tars with special characters work.

[tanabi]

@william8000 Awesome!!

[tanabi]

@mdadams @thargol @william8000 Hey guys. I was driving through to look at another PR and I see this one's still sitting. We should probably merge it.

[william8000]

I've been using it on my laptop without any problems. I have no plans on submitting more commits in the near future unless someone has a suggestion or problem with this PR.

[mdadams]

@thargol Just pinging you on this PR: If you have any concerns about the changes in this PR, please let me know by Friday this week. If I have not seen any concerns raised by that time, I will proceed to merge the PR so as not to hold up progress.

[thargol]

I've been too busy to properly review it, but from my cursory glance there doesn't seem to be anything there that would prevent it from being merged.

[mdadams]

@thargol I have just merged it, since it seems to look okay to you.

[william8000]

Thanks! Thanks also for merging Paul Howarth's PR. Having MAXPATHLEN different in some modules could have made memory overwrites possible, in addition to causing the link-time optimization issues that he noticed.

[mdadams]

@william8000 Thank you again for all of your changes. The HiDPI support that you added makes xv much more usable on high-resolution monitors.