This makes several calling sites shorter, and makes it easier
to add a miter limit with less plumbing in the future.
This also exposed a bug in AffineDisplayListPlayerCPU.cpp: We weren't
passing path linecap style on. I added a FIXME for that for now.
No intended behavior change.
Using the path rasterizer here is much slower than simply drawing four
lines. This also more accurately shows the (real) clip as the bounding
box is truncated to an int before adding it as a clip rect.
Fixes#23056
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.
