https://bugs.kde.org/show_bug.cgi?id=373897
Duncan <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #12 from Duncan <[email protected]> --- Two vital questions, and (as I review pre-posting) it seems I've written "a book" about the presumed answers! Oh, well... Take your pick of the fact it took a decade for someone who knew enough to care enough to post an explanation, or my ASD-driven detail compulsion, for the excuse. (Both?) 1) Does this bug occur only with jpeg images (I expect so, plus the attachments are all jpeg and I don't see anything else specifically mentioned)? 2) Does it reproduce on image resolutions (x or y) exactly divisible by 8 (under some conditions 16), or only if there's a remainder? I expect only with a remainder. If it's only jpeg and only with "remainder" resolutions as I strongly suspect, the bug... is complicated in that it's not primarily a gwenview bug, but rather derives from a rather technical libjpeg/libjpeg-turbo "feature", or as I'd be more likely to describe it as an affected user, an "unlikely to be fixed documented bug there for technical reasons." (Tho there are options to improve gwenview's handling of the problem, see the final two paragraphs.) [Perhaps interesting but non-vital personal case-related history:] I first came across this libjpeg(-turbo) problem I'd /guess/ about a decade ago, thus very likely roughly concurrent to the original filing of this bug, and actually filled in another piece of the puzzle (the 8x8 tiling bit) for myself only a year or two ago when I came across it while looking for something else (IIRC jpeg2k relationship to jpeg... as I was trying to decide whether to enable some related gentoo USE flag) in the wikipedia jpeg article. Beyond those clues I've only filled in the rest in the last couple hours (now four...) as I tried to turn the bits floating around in my head into something semi-coherent enough to put here, and I've redrafted several times already... Disclaimer: The following is certainly beyond my "technical comfort range" as while I'm a gentooer and admin of over two decades I'm neither a dev nor a jpeg/libjpegturbo expert, and any better informed correction is welcomed, but given the above mentioned decade it took to get even this hopefully somewhat well informed if only just now comment.. well we can still hope... The technical root of the problem is in how the various jpeg encodings break down an image into "tiles" (the libjpeg docs have a more technical label, iMCU, see the quote below) of 8x8 pixels, thus the origin of the 8 or 16 divisibility above. Also note that in some cases there's a scale factor which if I'm interpreting things correctly could multiply that as actually displayed by some factor at times (perhaps that's the origin of the 16, perhaps not?). Now (as I said) being neither a dev nor a libjpeg(-turbo) expert, the actual library documentation was a bit too dense to try to digest for this comment, as I was finding all sorts of /other/ stuff, but nothing directly related to this. Fortunately, however, libjpeg(-turbo) ships with a number of executable tool binaries that make use of the libs, along with their manpages, and it was in one of those manpages that I finally found confirmation and more detail of what I barely remembered from a decade ago or whenever it was... Specifically, if your distro ships the binaries and manpages as part of libjpeg-turbo (not split into another package; libjpeg-turbo being a dependency of gwenview so it should be installed if gwenview is), the following is a quote (reformatted and condensed) from the jpegtran (1) manpage: The image can be losslessly transformed by giving one of these switches: -flip horizontal/vertical -rotate 90/180/270 -transpose -transverse [transpositions across the diagonal axes] [Transpose] has no restrictions regarding image dimensions. [Now the *interesting* bit!] The other transformations operate rather oddly if the image dimensions are not a multiple of the iMCU size (usually 8 or 16 pixels), because [snip technical]. jpegtran’s default behavior [with] an odd-size image [preserves] exact reversibility[.] [Transpose] is able to flip the entire image area [without problems]. Horizontal mirroring leaves any partial iMCU column at the right edge untouched, but is able to flip all rows of the image. Similarly, vertical mirroring leaves any partial iMCU row at the bottom edge untouched, but is able to flip all columns. The other transforms can be built up as sequences of transpose and flip operations [so] their actions on edge pixels [are] the same as the end result of the corresponding transpose-and-flip sequence. [If you] prefer to discard any untransformable edge pixels [to having] a strange-looking strip along the right and/or bottom edges of a transformed image [add] the -trim switch: -trim Drop non-transformable edge blocks. Obviously, a transformation with -trim is not reversible, so strictly speaking jpegtran with this switch is not lossless. Also, the expected mathematical equivalences between the transformations no longer hold. For example, -rot 270 -trim trims only the bottom edge, but -rot 90 -trim followed by -rot 180 -trim trims both edges. -perfect [instead] causes jpegtran to fail with an error if the transformation is not perfect. End quote, tho there's (much) more to the manpage, including an example commandline suitable for scripting that (apparently, noting that I've not read the other manpages or indeed all of this one) can be used with -perfect and calls to the other tools to use jpegtran to do the lossless transform if possible, using the generated error condition if not to run a series of several commands including djpeg (decompress) and cjpeg (compress) that isn't lossless due to the full decompress/recompress cycle with the transform using another tool between, but eliminates the strip while maintaining resolution, at the cost of the quality loss of the normal jpeg recompression step. And as mentioned there's manpages for the other executables as well. So that's from the jpegtran (1) manpage covering the executable, which uses and ships with libjpeg(-turbo), but I couldn't see anything parallel for the library itself, which gwenview uses too. Still, it's exactly what we, or at least I, see from gwenview. But there are as yet at least two more (related) pieces of the puzzle I've not mentioned, both back on the gwenview side. To explore these one needs a copy of the gwenview sources, which, given I run gwenview (and indeed most of my kde install) built and routinely updated from live-git sources using the gentoo/kde overlay ebuilds for that purpose, I happen to have close at hand. =:^) The first of these two pieces... I'd expect gwenview to require libjpeg(-turbo) as a dependency (at both build and run time) since it links against it, and indeed, on gentoo at least, that is the case. However, looking at the gwenview sources, it includes not one but three partial copies of the libjpeg sources, 6.2 (as lib/libjpeg-62/), 8.0 and 9.0. Looks like these, there are (only) six files in each, three header files, transupp.c which is apparently non-header support code to be built as part of the lib-using executable, README.jpeg from the upstream library package (with license, etc, so covering that legal base), and a .clang-format file that disables that tool in that subdir. So the three are not full "vendored" (aka "bundled", possibly with code changes specific to the including app, and "interesting" situation for distros and security-aware admins should a security hole be found in the upstream library) library copies, just the headers and direct support code, I suppose for convenience as it likely allows at least limited functionality of various developer tools even if a full libjpeg(-turbo) copy isn't available as it would need to be for a full build and then at runtime as well. IOW basically a non-story, not the fully vendored library copies I had feared might be there. Still, it was interesting seeing the READMEs there, in triplicate but for the version, and find myself reading within them the exact same stuff I'd read in the actual library README on my system. And the last, related piece, before I summarize? lib/jpegcontent.cpp (and the related .h, and also lib/jpegerrormanager.h as well, all three files with namespace Gwenview so definitely gwenview code not libjpeg). So what's interesting about gwenview's lib/jpegcontent.cpp source file? Simply the following comment on line 491 (as of yesterday's head 5e775...): // The following code is inspired by jpegtran.c from the libjpeg OK, I think we've come full circle. The jpegtran manpage detailed that executable, not gwenview, but gwenview's "jpegcontent" code is based on it, and sure enough, as we know it has the same issue as jpegtran. While jpegtran, and gwenview's code based on it, may be nominally jpeg-process lossless as it avoids a full decode/recode cycle for its transforms, that comes at the cost of having to deal with jpeg's 8x8 tiles. That cost is paid when transforms are done on images at resolutions that don't evenly divide by 8 pixels in one or both directions. So jpegtran has its losslessly reversible but ugly strip default, an entirely logical default for an otherwise lossless transforming app since it maintains that losslessness in a severely technically challenging situation, but it also has the -trim and -perfect commandline options for those for whom the default isn't appropriate. For gwenview that same losslessly reversible but ugly choice is a bit less fortunate as it lacks those options, but looking at the alternatives I still believe it's the correct choice. Would silently destroying the data in that strip (jpegtran's -trim option) be a better alternative? I can't see how. What about refusing to do the transform at all if the image resolution wasn't appropriately 8 or 16-divisible even? That's going to result in frustrated users too! So sticking with jpegtran's ugly but reversible default given it's the only available option does indeed seem the sane if ugly alternative -- **IF** we accept the condition of sticking within the box of those three choices. As implied I think gwenview can do better, now that we have a reasonable (if lengthy) presentation of the facts at hand. At minimum, I think a short popup warning referring to a page in gwenview's help or some link for more information would be appropriate, before doing an operation that might result in "stripping". With a "Go ahead, I'll take a strip if it results" and a "No I don't want that, Cancel", as exit buttons on the dialog, along with its link to more info. Better, I think, would be an option setting (well, two) in gwenview's configuration. The first setting would be a trinary consisting of the three options the jpegtran commandline offers, only here presented as a dropdown or radio button, that choose which option gwenview transforms use: 1) lossless but ugly strip (default), 2) strip-losing non-reversible trim, 3) refuse the transform if it can't be done perfectly. The (related but separate) second setting would effectively be a "don't ask me again" checkbox, either implemented on the warning dialog itself, or as an "always warn first" option in the gwenview config along side the first trinary, so people could decide to always be warned if they want to be cautious or need to change the default often, or never be warned if they are comfortable with their config setting and their ability to find it again should they want to change it. Then the warning dialog, if not turned off, could reflect the config trinary choice setting it'll use, and tell people where the setting is located if they want to cancel the warning and possibly change the config before trying again. -- You are receiving this mail because: You are watching all bug changes.
