...as long as fewer than 32K entries are referenced in a single
text region segment.
Having more than 32K entries in a symbol dictionary happens in some
real-world PDFs where the "global" JBIG2 stream that's shared over
all pages has lots of letters. But the text region segment on every
single page just has a couple thousand letters (160 characters per line
at 100 lines and a couple thousand dust speckles is well under 32K
symbol references per page).
I'd like to add a test that's similar to such a file, so we need to
support writing text regions referring to symbol dictionaries with
more than 32K symbols.
A test jbig2 image I want to create will have a height class bitmap
that's 9 * 2 ** 18 = 2,359,296 pixels wide (and 2 high).
So bump up our limit on bitmap width and height from 64 ki to 4 Mi.
The latest version of Clang got better at detecting unused templates. It
detected one truly unused template in our codebase. The rest are either
from templated functions that are not always used (depending on the
compile-time configuration) or from templated functions that are
incorrectly annotated with static.
A signed DIB height may be negative for top-down images, but the
minimum signed value cannot be negated. Reject it while parsing so
size and frame decoding cannot overflow.
This is a slight behavior change in some places. Some places did
(r.size().to_type<float>() * max(scale_x, scale_y)).to_type<int>();
After this commit, the semantics are now like `(...).to_rounded<int>()`,
that is we no longer sometimes omit a column or row in the fitted
direction.
scaled_to_fit_within() is like CSS's `object-fit: contain`: It returns
the largest rect that completely fits into the given rect, with an exact
fit in one dimension, and centered in the other dimension.
scaled_to_cover() is like `object-fit: cover`: It returns the smallest
rect that completely covers the given rect, with an exact fit in one
dimension, and centered in the other dimension.
Takes `Build/lagom/bin/image -o out.tiff ~/Downloads/balloon.jpf` from
531.4 ms ± 5.4 ms to 515.8 ms ± 4.7 ms ms under hyperfine on my system
(2.7% less). When run under `/usr/bin/time -l`, the changes are:
Before:
13202665108 instructions retired
2281080551 cycles elapsed
After:
12821908788 instructions retired
2213264986 cycles elapsed
Only affects lossy files.
Takes `Build/lagom/bin/image -o out.tiff ~/Downloads/balloon.jpf` from
542.3 ms ± 7.0 ms to 527.2 ms ± 4.7 ms under hyperfine on my system
(2.7% less). When run under `/usr/bin/time -l`, the changes are:
Before:
13956342230 instructions retired
2322276362 cycles elapsed
After:
13251294364 instructions retired
2270195534 cycles elapsed
The absolute win on the lossless file is again a bit smaller
(due to smaller image dimensions), and the relative win is tiny
there (due to it spending much more time in bitplane decoding and
less in IDWT).
Takes `Build/lagom/bin/image -o out.tiff ~/Downloads/balloon.jpf` from
586.0 ms ± 5.6 ms to 547.7 ms ± 5.1 ms under hyperfine on my system
(6.6% less). When run under `/usr/bin/time -l`, the changes are:
Before:
14557698316 instructions retired
2500654210 cycles elapsed
After:
13958498350 instructions retired
2318667968 cycles elapsed
The lossless files goes from 1.025 s to 1.000 s (likely smaller win
because it's half the dimensions).
(Note that the "from" hyperfine number is pretty different from the
previous commit, probably due to my system being under different load,
at different battery level, or what. But the "instructions retired"
are very comparable, so that's probably a better number to look at.)
Slight subtlety: Forming the pointer
`data() + (buffers.scanline_start - i0)` is usually technically invalid
in C++: scanline_start is usually 4 and i0 can be up to the current
rect's left or top, so it'll usually point to before the image.
I haven't seen this cause actual problems in practice, but make the
buffers max(left,top) longe, even if that leading data ine is never
touched.
Takes `Build/lagom/bin/image -o out.tiff ~/Downloads/balloon.jpf` from
704.3 ms ± 5.4 ms to 613.2 ms ± 3.9 ms under hyperfine on my system
(12.9% less). When run under `/usr/bin/time -l`, the changes are:
Before:
18035577126 instructions retired
2862272914 cycles elapsed
After:
14521350138 instructions retired
2492032301 cycles elapsed
balloon.jps uses the lossy 9-7 transform. For lossless files, this helps
too, but less, as the 5-3 transform uses fewer loops in this function.
Also, lossless files spend relatively much more time in bitplane
decoding and less in IDWT, so the relative win for them is smaller too:
For ballon_rev_half.jp2, which is balloon.jpf resized to 50% and
reencoded in lossless, from 1.058 s to 1.035 s.
I'd like to make changes to our JPEG2000 IDWT implementation to speed
it up. At the moment, it's very close to the spec, which seems useful
for correctness and understandability.
So copy the existing file, put the code in there in a Reference
namespace, and in JPEG2000_DEBUG builds, compare the output of the
two implementations. (At the moment, both implementations are
identical, but that will change more and more over time.)
That way, we can run `Meta/test_pdf.py` with a large test corpus with
the comparison turned on to make sure IDWT optimizations aren't
completely broken.
Run `git show` with -C / --find-copies on this commit to make it display
the new file as copy of the old one, along with changes I made to it.
(Namely: I removed the input structs from the reference file as they're
shared with the production file, and I qualified a few calls with
`Reference::`.)
No behavior change.
step_size is identical for all coefficients. No behavior change.
Takes
hyperfine \
'Build/lagom/bin/image --no-output ~/Downloads/balloon.jpf'
from 741.7 ms ± 8.0 ms to 648.0 ms ± 1.9 ms on my system,
a 12.5% reduction.
This is a prerequisite to allow running GUI apps on non-serenity systems
where `/res` is not a valid path. It defers to the loaded resource
implementation, which by default loads from file.
...and in a few cpp files too, and clean up missing includes.
Reduces number of files recompiled after touching AK/Math/Hyperbolic.h
from a couple thousand to a handful.
No behavior change.
When choosing the best image from an ICO file, prefer the entry with the
largest pixel area first, and only use bits-per-pixel as a tie-breaker.
This had regressed in commit b10fe7c136.
This fixes ICO files like Discord's favicon, where 16x16, 32x32, 48x48,
and 256x256 entries all advertise the same bpp. We were previously just
choosing whichever we saw first, which happened to be the 16x16 icon.
(cherry picked from commit b7c4cd511f0f1b02b90dc7be66333617e5a84cb3)
VP8Header now has an optional field with the keyframe data.
Move rejecting of non-keyframes out of decode_webp_chunk_VP8_contents()
into WebPLoader.cpp, decode_webp_first_chunk().
In VP8/Decoder.cpp, we can now check if that optional has a value to
detect if a frame is a keyframe, instead of having to rely on
decode_webp_chunk_VP8_contents() returning an error.
No real behavior change.
d1b5eec154 added first_partition / second_partition fields,
so it's no longer necessary to store size_of_first_partition here.
It's identical to first_partition.size(), and nothing reads it.
No behavior change.
The idea is described here https://kornel.ski/deringing/. And allows
reducing the noise around sharp white edges. This is visible when
encoding an image like `buggie.png`. One nice aspect of the optimization
is that it only affects macroblocks where it can help.
Unlike the previous commit, IsUnsigned is never true for floating-point
types, since there are no unsigned floats.
However, it's still nice to keep the intent clear that this concept
is only for integral types.
argyll's `targen` writes '*nix' to this value. This is not allowed
per spec as far as I understand (see included link), but argyll-
created profiles are encountered in the wild, for example here:
https://www.thregr.org/wavexx/rnd/20260201-remarkable_pro_colors/#downloadable-files
So allow it. With this, `icc` can dump the profile at that URL.
The v2 spec allows 'TGNT', so allow that too while here.
Before this change:
% hyperfine 'Build/lagom/bin/image --no-output tmp.tiff'
Benchmark 1: Build/lagom/bin/image --no-output tmp.tiff
Time (mean ± σ): 245.3 ms ± 1.6 ms
[User: 236.7 ms, System: 7.4 ms]
Range (min … max): 243.8 ms … 248.6 ms 12 runs
% hyperfine 'Build/lagom/bin/image -o tmp2.tiff tmp.tiff'
Benchmark 1: Build/lagom/bin/image -o tmp2.tiff tmp.tiff
Time (mean ± σ): 544.8 ms ± 7.6 ms
[User: 525.9 ms, System: 16.0 ms]
Range (min … max): 539.4 ms … 565.7 ms 10 runs
After this change (--no-output time does not change):
% hyperfine 'Build/lagom/bin/image -o tmp2.tiff tmp.tiff'
Benchmark 1: Build/lagom/bin/image -o tmp2.tiff tmp.tiff
Time (mean ± σ): 266.6 ms ± 1.6 ms
[User: 251.9 ms, System: 13.5 ms]
Range (min … max): 264.8 ms … 269.3 ms 11 runs
tmp.tiff is a 4096x4096 RGB TIFF file.
So the time spent writing a 4096x4096 RGB TIFF file goes from
~300ms to ~20ms, over 10x as fast.
image.cpp does use a OutputBufferedFile, but calling
stream.write_value<u8>() almost 50 million times (4096x4096x3)
at 5.8 ns does explain that overhead. And OutputBufferedFile
does need a virtual call, bounds check at each call, and
TRY() handling at each call, so it seems believable that its
enough to create that much overhead.
(Making a buffer the size of the whole data and calling
write_until_depleted() just once at the end even gets to 250 ms,
but this here seems like a nice time/memory tradeoff.)
No behavior change.
Reduces the time for
Build/lagom/bin/image --no-output \
--convert-to-color-profile sRGB out-cmyk.tiff
from 3.2s to 2.5s on my machine, a 22% speedup (!).
out-cmyk.tiff is one of the CMYK outputs described in
https://github.com/SerenityOS/serenity/pull/26459#issuecomment-3619058241
Before, the lambdas got passed as an AK::Fuction, and the optimizer
apparently isn't able to undo that after inlining, requiring a virtual
dispatch per call.
The writer wrote code lengths for all used symbol IDs, but the
spec wants us to write a code length for all referenced symbols.
The loader already got this right, but the writer didn't. (I noticed
this while creating the previous test case and locally futzed around
with symbol IDs.)
While here, also check that all symbol IDs in a text region are
in bounds.
When we don't export some symbols, symbol indices are different in
different symbol dictionary sections.
For example, in the included test, segment 1 exports 7 symbols, and
segment 2 refers to it, but only exports 3 of these 7 symbols, and it
only exports 1 of the 4 symbols itself defines. So within segment 2,
indices 0-10 are valid. But in segment 3, which refers to segment 2,
only indices 0-3 are valid, and they refer to the symbols that have
indices 0, 1, 6, and 10 in segment 2.
This is mostly a problem for the writer (see #26510 / d15f57455a).
When the text region refines its symbol 3, the code used to look up the
symbol ids that it referred to. But those indices were in the namespace
of segment 2, but we used the symbol list from segment 3.
As a fix, don't store symbol dictionary results as a
SymbolDictionarySegmentData::HeightClass::Symbol but just as a
BilevelImage. For regular symbols and regular refined symbols, it
doesn't make a difference. Strip-refined symbols are converted to
bitmaps eagerly instead of only when they are refined, but in return
this needs less code, it handles indices correctly, and chains of
strip-refined symbols are no longer O(n^2) (...not that these happen in
practice).
The new test is the same as bitmap-symbol-symbolrefine-textrefine.json
(or bitmap-symbol-symhuffrefine-textrefine.json), but the last
symbol_region sets `export_flags_for_referred_to_symbols` to not export
temporary symbols from its referred-to symbol dictionaries, and it sets
`"exported"` to false for its internal symbols (and symbol_ids are
adjusted to match).
Per format_overview.md:
> The codestream can optionally be split into multiple jxlp boxes;
> conceptually, this is equivalent to a single jxlc box that contains
> the concatenation of all partial codestream boxes.
It looks like the data is preceded by a u32 that's incrementing,
and has its highest bit set at the end.
Naively implement this by just appending the data from all jxlp
into a single vector.
With this, decoding the output of
cjxl Tests/LibGfx/test-inputs/jpg/big_image.jpg big_image.jxl
gets a bit farther.
With this, `isobmff` can dump the signature contents of a `JXL `
signature box.
JPEGXLImageDecoderPlugin::sniff() already checks for the right contents
of `signature`, so it's fine to remove the FIXME.
On average, this patch reduces the error in luminosity between an input
image and its dithered equivalent. This is done by correcting an off-by-
one error in the threshold computation and also by making sure the range
of mapped gray values to matrix pattern is of equal size for all ranges.
This means that all values between 0 and 255 / (N * N + 1) (51 for
Bayer2x2) will result in a black pixel while only a value of 0 would
previously do it.
The test works by first generating gray bitmap, then applying Bayer
dithering on them and finally compare the luminosity of the two.
This adds writer support for refinement of a symbol dictionary entry
that is itself a strip refinement.
This case is a bit awkward for the writer: To write the refinement, we
need the bitmap that's being refined. But since that's a strip refine,
we don't have that bitmap, we only have the parts it consists of.
As #26411 explains, the approach of calling into the loader to do
the strip decoding doesn't work here: We need the decoded strip
refinement while writing the strip itself. So we'd have to send
a partially encoded strip to the loader, which isn't something we
can currently do.
But since strip refinements only use a limited subset of full text
strips (always a background color of 0, always a reference corner
of topleft, always a composition operator of "or"), reimplementing
the loader's logic for this case, while conceptually not nice, is
not a lot of code. So let's just do that.
Using this, also add a test for this feature. I believe this tests
the last missing refinement case :^)
The test uses regular symbols for the eyes (symbols 0 and 1) and the
mouth (symbol 6).
The nose, 100x100 pixels, is conceptually split into 60x60, 40x60,
60x40, 40x40 tiles. Since symbols are sorted by increasing height, the
latter two are symbols 2 and 3, the former are symbols 4 and 5.
Symbol 3 isn't actually the bottom right part of the nose, but
contains the pixel data of an eye. This is for testing refinement
below.
To test strip background color filling, the 60x60 tile is actually
59x60 pixels. And to test refinement, the top left tile is actually
solid white instead of the top left part of the nose.
The first refinement symbol (id 7) refines the top left tile to a 59x60
tile that's the top left corner of the nose, but shifted several pixels
to the right.
The second refinement symbol (id 8) is a strip refinement. It puts
symbols 7 and 5 next to each other, to produce the top half of the nose
(with the left part being shifted to the side).
The third refinement symbol (id 9) is a simple refinement of symbol 8
refines it to the actual top of the nose (to fix the shift, and to
test refinement of strips in a symbol dictionary).
The fourth refinement symbol (id 10) is a strip refinement that puts
symbol 9 in the first strip, with an offset of 1, and symbols 2 and 3
in a second strip. This means symbol 10 is almost the nose, but its
bottom right contains the pixel data of an eye, since that's what
symbol 3 contained.
Finally, the text region refines symbol 10 to the actual nose pixels,
to test refinement of strips from a text region.
Whew!
Since the text region refines to the nose, it's useful to locally remove
that refinement to test that the reference bitmap looks as expected.
Similarly, it's useful to locally make the final strip refinement put
symbol 8 instead of symbol 9 in the first strip, to make sure the input
to symbol 9 looks as expected.
The test file decodes fine in all viewers I've tried (PDFium,
Preview.app, pdf.js, mutool). (This case is much more awkward for the
writer than for the loader, after all.)
Since it's easy to do now, only allow references to dead pattern
dictionary segments in Power JBIG2 files, not to all segments.
That's enough to still be able to decode all Power JBIG2 files.
(Except for files 042_13.jb2, 042_14.jb2, which didn't decode before
either, and which don't decode in any viewer that I tried.)
jbig2enc, which is used by Google Books, sometimes writes files that
are technically invalid. Allow that in permissive mode.
To be able to check the referred-to segment's types, switch the loop
from iterating over header.referred_to_segment_numbers to iterating
over referred_to_segments directly (possible after the third commit
of #26191, 72cd151fb9).
Fixes#26405.
Clients of JBIG2Loader can now pick how strictly invalid images will
be rejected.
By default, it's set to permissive, which allows spec violations that
happen in practice and are easy to work around. At the moment, this
is just for images in the Power JBIG2 suite, but this will be used to
relax some other checks soon.
jbig2-from-json now requests strict spec-compliant checking. This for
now has the effect that manually adding a comment advertising a file
as being a Power JBIG2 file will no longer allow a few quirks when
using jbig2-from-json.
A future commit will optionally make JBIG2Loader more strict,
and then this will be false even for Power JBIG2 files. So
`allow_power_jbig2_quirks` seems like a better name.
As far as I can tell, nothing in the spec forbids symbol dictionaries to
have 0 symbols, or text regions to not use any symbols.
Section 7.3.1 even explicitly says that text regions 'may refer to any
number of segments of type "symbol dictionary"', which sounds like it
includes 0.
The spec does say that ceil(log2(num_symbols)) should be computed
though, which is undefined if num_symbols is 0. But ceil(log2(1)) is
0, and using 0 for ceil(log2(0)) feels like what the spec intends.
Furthermore, this seems to be the behavior of other jbig2 decoders.
ceil_log2(x) is 0 for x == 0, so just switching from ceil(log(x))
to AK::ceil_log(x) has the effect that we want.
The test has one text region referring to a symbol dictionary without
symbols, and a text region not referring to any symbol dictionaries,
to make sure that those are drawn without exceptions.
Finally, it has a generic region with the actual image data on top.
ceil_log2(x) is like ceil(log2(x)) for integer types. One difference
is that ceil_log2(x) is 0 for x == 0, while ceil(log(x)) is not defined
for x == 0.
In these cases, x is guaranteed to be > 0, so both should be equivalent
here.
No behavior change.
A halftone region's graymap stores an index that picks a bitmap
from the pattern dictionary. For each graymap pixel, an affine transform
maps that pixel to a position in the output image, and the pattern is
drawn at that position.
So far, we only supported an identity grid that neatly tiled the pattern
bitmaps across the image.
Now we support the full affine transform. This allows for a rotated
grid (for the top left positions; the pattern bitmaps are always drawn
axis-aligned from the top left position).
For tile generation for tests, we just collect tiles off that grid,
and then match against those in graymap generation. This means the
test json file needs to contain numbers that make sense: We currently
don't automatically compute a good rotated grid that covers the
output image.
Tile generation for pattern images clipped on the left or right is
probably wrong. Since the test image has a generous white border,
it doesn't negatively affect our tests, but it's something we might
want to improve in the future.
For `pattern_dictionary`, if the grid parameters are omitted, they
default to an identity grid. Else, it's up to the author of the
json file to make sure the parameters match what's used in the
corresponding halftone region.
For rotated grids, the pattern bitmaps need to overlap to not
have gaps. The pattern construction method currently assumes
that `combination_operator` of `or` is used for the halftone
region.
Skip patterns are really only interesting for halftone regions with
a rotated grid, which we can't do yet. So the skip pattern will be
all "don't skip" entries everywhere for now.
But once we do add support for non-identity grids, this will hopefully
just work. And even with an identity grid, this already produces a
useful test file.
Similar to what #26452 did for Lut8TagData and #26462 for Lut16TagData.
With this, we can convert from PCS to all tag types :^)
Which means our ICC implementation is now fairly complete! We don't
support DeviceLink or Abstract profile types (both don't appear in
images; both are arguably useful in some not-embedded-in-images
scenarios), nor the optiona BToD / DToB tags, and are missing
some other minor things (like grayscale profiles -- but not much
is missing here). But overall, this is now a fairly complete (if slow)
basic ICC implementation :^)
These are really only useful for testing, so don't mention them
in `icc --help` output for `-n`.
They're supposed to behave identically to the built-in XYZ and
LAB profiles (but not the LAB_mft2 profile, which uses a slightly
different internal encoding, see #26462. The new XYZ profiles added
here behave like the XYZ profile described there, but the new LAB
profiles do not behave like the LAB_mft2 profile described there,
but like the LAB (no _mft2) profile:
% echo 'pcslab(100, 127, -128)' | \
icc -n LAB_mABmBA_u8_clut --stdin-u8-from-pcs | \
icc -n LAB --stdin-u8-to-pcs
pcslab(100, 127, -128)
% echo 'pcslab(100, 127, -128)' | \
icc -n LAB_mABmBA_u16_clut --stdin-u8-from-pcs | \
icc -n LAB --stdin-u8-to-pcs
pcslab(100, 127, -128)
% echo 'pcslab(100, 127, -128)' | \
icc -n LAB_mABmBA_no_clut --stdin-u8-from-pcs | \
icc -n LAB --stdin-u8-to-pcs
pcslab(100, 127, -128)
The profiles without a CLUT already round-trip fine, so add
tests for those.
Nico Weber