These changes are compatible with clang-format 16 and will be mandatory
when we eventually bump clang-format version. So, since there are no
real downsides, let's commit them now.
We used to expand every bit in an 1bpp image to a 0 or 255 byte,
then map that to a float that's either 0.0f or 1.0f (or whatever's
in /DecodeArray), then multiply that by 255.0f to convert it to a
u8 and put that in the rgb channels of a Color.
Now we precompute the two possible outcomes (almost always Black
and White) and do a per-bit lookup.
Reduces time for
Build/lagom/bin/pdf --render-bench --render-repeats 20 --page 36 \
~/Downloads/Flatland.pdf
(the "decoded data cached" case) from 3.3s to 1.1s on my system.
Reduces time for
Build/lagom/bin/pdf --debugging-stats ~/Downloads/0000/0000231.pdf
(the "need to decode each page" case) from 52s to 43s on my machine.
Also makes paging through PDFs that contain a 1700x2200 pixel CCITT
or JBIG2 bitmap on each page noticeably snappier.
Rather than make path segments virtual and refcounted let's store
`Gfx::Path`s as a list of `FloatPoints` and a separate list of commands.
This reduces the size of paths, for example, a `MoveTo` goes from 24
bytes to 9 bytes (one point + a single byte command), and removes a
layer of indirection when accessing segments. A nice little bonus is
transforming a path can now be done by applying the transform to all
points in the path (without looking at the commands).
Alongside this there's been a few minor API changes:
- `path.segments()` has been removed
* All current uses could be replaced by a new `path.is_empty()` API
* There's also now an iterator for looping over `Gfx::Path` segments
- `path.add_path(other_path)` has been removed
* This was a duplicate of `path.append_path(other_path)`
- `path.ensure_subpath(point)` has been removed
* Had one use and is equivalent to an `is_empty()` check + `move_to()`
- `path.close()` and `path.close_all_subpaths()` assume an implicit
`moveto 0,0` if there's no `moveto` at the start of a path (for
consistency with `path.segmentize_path()`).
Only the last point could change behaviour (though in LibWeb/SVGs all
paths start with a `moveto` as per the spec, it's only possible to
construct a path without a starting `moveto` via LibGfx APIs).
Fixes pages 17-19 on
https://www.iro.umontreal.ca/~feeley/papers/ChevalierBoisvertFeeleyECOOP15.pdf
Calling the fill handler after painting the stroke as previously doesn't
work, since we need to set up the clip before both stroke and fill, and
unset it after both. The duplication is a bit unfortunate, but also
minor.
An array image mask contains a min/max range for each channel,
and if each channel of a given pixel is in that channel's range,
that pixel is masked out (i.e. transparent). (It's similar to
having a single color or palette index be transparent, but it
supports a range of transparent colors if desired.)
What makes this a bit awkward is that the range is relative to the
origin bits per pixel and the inputs to the image's color space.
So an indexed (palettized) image with 4bpp has a 2-element mask
array where both entries are between 0 and 15.
We currently apply masks after converting images to a Gfx::Bitmap,
that is after converting to 8bpp sRGB. And we do this by mapping
everything to 8bpp very early on in load_image().
This leaves us with a bunch of options that are all a bit awkward:
1. Make load_image() store the up- (or for 16bpp inputs, down-)
sampled-to-8bpp pixel data. And also return if we expanded the
pixel range while resampling (for color values) or not (for
palettized images). Then, when applying the image filter,
resample the array bounds in exactly the same way. This requires
passing around more stuff.
2. Like 1, but pass in the mask array to load_image() and apply
the mask right there and then. This means we'd apply mask arrays
at a different time than other masks.
3. Make the function that computes the mask from the mask array
work from the original, unprocessed image data. This is the most
local change, but probably also requires the largest amount of
code (in return, the color mask for 16bpp images is precise, in
addition that it separates concerns the most nicely).
This goes with 3 for now.
Makes text show up on 0000646.pdf pages 87-92, which for some reason
renders all text using 2x2 images with huge masks that contain
rendered text outlines.
We still don't apply it to the glyph itself, so they don't show up
scaled or rotated, but they're at the right spot now.
One big thing this here hsa going for it is that the final glyph
position is now calculated with just
`ext_rendering_matrix.map(glyph_position)`.
Also, character_spacing and word_spacing are now used unmodified
in the SimpleFont::draw_string() loop. This also means we no longer
have to undo a scale when updating the position in
`Renderer::show_text()`.
Most of the rest stays pretty yucky though. The root cause of many
problems is that ScaledFont has its rendering sized baked into the
object. We want to render fonts at size font_size times scale from
text matrix times scale from current transformation matrix (but
not size from hotizontal_scaling). So we have to make that the
font_size, but then we have to undo that in a bunch of places to
get the actualy font size.
This will eventually get better when LibPDF moves off ScaledFont.
Fonts should have size font_size times total scaling. We tried to
get that by computing text_rendering_matrix.x_scale() * font_size,
but text_rendering_matrix.x_scale() also includes
horizontal_scaling, which shouldn't be part of font size.
Same for character_spacing and word_spacing.
This is all a big mess that's caused by LibPDF using ScaledFont,
which requires scaling to be aprt of the text type. I have an
in-progress local branch that moves LibPDF to directly use VectorFont,
which will hopefully make this (and other things) nicer. But first,
let's get this right, and then make sure we don't regress it when
things change :^)
While PDFFont::draw_string() already returns a position scaled by
horizontal_scaling, the division by text_rendering_matrix.x_scale()
(which also contains the scaling factor) undid it. Reapply it.
Fixes the horizontal layout of the line
"should be the same on all lines: super" in Tests/LibPDF/text.pdf.
PDF spec 1.7 5.3.3 Text Space Details gives the correct multiplication
order: parameters * textmatrix * ctm.
We used to do text * ctm * parameters
(AffineTransform::multiply() does left-multiplication).
This only matters if `text_state().rise` is non-zero. In practice,
it's almost always zero, in which case the paramter matrix is a
diagonal matrix that commutes.
Fixes the horizontal offset of "super" in Tests/LibPDF/text.pdf.
Since we can't clip against a general path yet, this clips images
against the bounding box of the current clip path as well.
Clips for images are often rectangular, so this works out well.
(We wastefully still decode and color-convert the entire image.
In a follow-up, we could consider only converting the unclipped
part.)
Turns out the width/height in a `re` command can be negative. This
results in rectangles with different winding orders. For example, a
negative width results in a reversed winding order.
Previously, this was lost by passing the rect through an
`AffineTransform` before constructing the path. So instead, this
constructs the rect path, and then transforms the resulting path.
Mostly because I audited all places that assigned to `m_text_matrix`
after #22760.
This one is very difficult to trigger in practice.
`show_text()` marks the text rendering matrix dirty already,
so this only has an effect if the `TJ` array starts with a
number, and the matrix isn't marked dirty going in.
`Tm` caches the text rendering matrix, so I changed text.pdf
to contain:
```
1 0 0 1 45 130 Tm
[ 200 (Hello) -2000 (World) ] TJ T*
```
This first sets an x offset of 5 (on top of the normal 40), and
then undoes it (`200` is multiplied by font size (25) / -1000,
and `200 * 25 / -1000` is -5). Before this change, the topmost
"Hello World" ended up slightly indented.
Likely no behavior change in practice, but makes the code easier
to understand, and maybe it helps in the wild somewhere.
0000342.pdf page 5 contains this snippet:
```
/T1_1 10.976 Tf
0 -31.643 TD
(This)Tj
1 0 0 1 54 745.563 Tm
22.181 -31.643 Td
[(vehicle)-270.926(uses)...
```
The `Tm` marked the text rendering matrix as dirty at the start,
but it then calls calculate_text_rendering_matrix() almost in the
next line, which recalculates the text rendering matrix and caches
the new matrix. The `Td` used to not mark it as dirty, and we'd
draw "vehicle" with an incorrect matrix.
All ColorSpace subclasses converted to float anyways, and this
allows us to save lots of float->Value->float conversions during
image color space processing.
A bit faster:
```
N Min Max Median Avg Stddev
x 50 0.99054313 1.0412271 0.99933481 1.0052408 0.012931916
+ 50 0.97073889 1.0075941 0.97849107 0.98184034 0.0090329046
Difference at 95.0% confidence
-0.0234004 +/- 0.00442595
-2.32785% +/- 0.440287%
(Student's t, pooled s = 0.0111541)
```
A certain PDF was drawing some text used `9 0 0 9 474.54 700.6801 Tm`
to set the text matrix to a matrix that scaled by 9 in one text object.
Then, after ending that text object, it had the following new text
object which contained nothing that invalidated the text matrix:
```
BT
/F1 7 Tf
/DeviceRGB CS
0 0 0 SC
10 TL
86.37849 21.908 Td
(Authorized licensed use limited to: ...) Tj
ET
```
`BT` did reset it as required, but since we didn't mark the matrix
as dirty, we never recomputed it and drew the additional text scaled
up 9x.
An image mask is a 1-bit-per-pixel bitmap that's black where the
current color should be painted, and white where it should be
transparent (think: like ink).
load_image() already converts images like this into 8-bit-per-pixel
images that have 0xff, 0xff, 0xff in rgb for opaque (originally 0 bit)
pixels and 0, 0, 0 in rgb for transparent pixels.
So we just move copy the image mask's image data into the alpha
channel and replace rgb with the current color, and then draw
it like a regular bitmap.
The idea is to massage the inline image data into something that
looks like a regular image, and then use the normal image drawing code:
We translate the inline image abbreviations to the expanded version at
rendering time, then unfilter (i.e. uncompress) the image data at
rendering time, and the go down the usual image drawing path.
Normal streams are unfiltered when they're first accessed, but
inline image streams live in a page's drawing operators, and this
fits the current approach of parsing a page's operators anew
every time the page is rendered.
(We also need to add some special-case handling for color spaces
of inline images: Inline images can use named color spaces, while
regular images always use direct color space objects.)
We create a inline_image_end operator that has all the relevant data
in a synthetic StreamObject.
inline_image_end is still a RENDERER_TODO(), so no real behavior
change. (Previously we'd call only inline_image_begin, so string the
todo message is about is now a bit different. But no interesting
behavior change.)
This commit un-deprecates DeprecatedString, and repurposes it as a byte
string.
As the null state has already been removed, there are no other
particularly hairy blockers in repurposing this type as a byte string
(what it _really_ is).
This commit is auto-generated:
$ xs=$(ack -l \bDeprecatedString\b\|deprecated_string AK Userland \
Meta Ports Ladybird Tests Kernel)
$ perl -pie 's/\bDeprecatedString\b/ByteString/g;
s/deprecated_string/byte_string/g' $xs
$ clang-format --style=file -i \
$(git diff --name-only | grep \.cpp\|\.h)
$ gn format $(git ls-files '*.gn' '*.gni')
