Adrian Johnson discovered cases where we mistakenly compared the result
of unsigned arithmetic where we need signed quantities. Look for similar
cases in the users of cairo_rectangle_int_t.
After discussing the scaled font locking with Behdad, it transpired that it
is not sufficient for a font to be locked for the lifetime of a scaled glyph,
but that the scaled font's glyph cache must be frozen for the glyph'
lifetime. If the cache is not frozen, then there is a possibility that the
glyph may be evicted before the reference goes out of scope i.e. the glyph
becomes invalid whilst we are trying to use it.
Since the freezing of the cache is the stronger barrier, we remove the
locking/unlocking of the mutex from the backends and instead move the
mutex acquisition into the freeze/thaw routines. Then update the rule on
acquiring glyphs to enforce that the cache is frozen and review the usage
of freeze/thaw by all the backends to ensure that the cache is frozen for
the lifetime of the glyph.
A little bit of sleep and reflection suggested that the use of
device_offset_[xy] was confusing and clone_offset_[xy] more consistent
with the function naming.
If the operator is unbounded, then its area of effect extends beyond
the definition of the mask by the trapezoids and so we must always perform
the image composition.
Fixes test/operator*.
Previously the rule for clone_similar() was that the returned surface
had exactly the same size as the original, but only the contents within
the region of interest needed to be copied. This caused failures for very
large images in the xlib-backend (see test/large-source).
The obvious solution to allow cloning only the region of interest seemed
to be to simply set the device offset on the cloned surface. However, this
fails as a) nothing respects the device offset on the surface at that
layer in the compositing stack and b) possibly returning references to the
original source surface provides further confusion by mixing in another
source of device offset.
The second method was to add extra out parameters so that the
device offset could be returned separately and, for example, mixed into
the pattern matrix. Not as elegant, a couple of extra warts to the
interface, but it works - one less XFAIL...
Use the utility functions _cairo_box_from_rectangle and
_cairo_box_round_to_rectangle() instead of open-coding. Simultaneously
tweak the whitespace so that all users of traps look similar.
Avoid tessellating the path if we know that the target extents is zero.
Besides the rare occurrence when everything is clipped out, a zero-sized
surface is often intended as a no-op surface for benchmarking.
Instead of allocating the union of all possible pattern types, just
allocate the specific pattern as used by the function in order to trim
the stack space consumption and flag potential misuse.
cairo_rectangle_int16_t was being used in a number of places instead
of cairo_rectangle_int_t, which led to memory corruption when cairo was
using a fixed point format with a bigger space than 16.16 (such as 24.8).
&image_extra was being passed instead of image_extra to release; the
bug only manifested itself when the particular backend did something
with image_extra.
Every time we assign or return a hard-coded error status wrap that value
with a call to _cairo_error(). So the idiom becomes:
status = _cairo_error (CAIRO_STATUS_NO_MEMORY);
or
return _cairo_error (CAIRO_STATUS_INVALID_DASH);
This ensures that a breakpoint placed on _cairo_error() will trigger
immediately cairo detects the error.
This reverts commit 919bea6dbb.
Sadly as Behdad points out some backends do modify the glyph array and,
for example cairo-xlib-surface, hide this from the compiler with some
evil casts.
Skip the memory duplication of the incoming glyphs if we do not need
to transform them into the backend coordinate system.
As a consequence we need to constify the glyphs passed to the backend
functions.
Introduce cairo_gradient_stop_t, and remove pixman dependency
for core pattern types. Perform conversion from cairo types
to pixman types as necessary in fallback code.
This patch introduces three macros: _cairo_malloc_ab,
_cairo_malloc_abc, _cairo_malloc_ab_plus_c and replaces various calls
to malloc(a*b), malloc(a*b*c), and malloc(a*b+c) with them. The macros
return NULL if int overflow would occur during the allocation. See
CODING_STYLE for more information.
It's quite simple to add a new _cairo_traps_limit call which installs
a box into the cairo_traps_t structure. Then at the time of
_cairo_traps_add we can discard any trapezoid that is wholly outside
the box and also clip any trapezoid that is partially outside the box.
We take advantage of this for both cairo_stroke and cairo_fill, (when
cairo is computing the trapezoids in cairo-surface-fallback.c). Note
that we explicitly do not do any clipping for cairo_stroke_extents,
cairo_fill_extents, cairo_in_stroke, or cairo_in_fill which are
defined to ignore clipping.
As seen by the long-lines perf case, this fix successfully works
around the bug in the X server where it creates overly large masks for
partially-outside-the-destination-surface trapezoids:
xlib-rgba long-lines-uncropped-100 545.84 -> 5.83: 93.09x speedup
██████████████████████████████████████████████
xlib-rgb long-lines-uncropped-100 554.74 -> 8.10: 69.04x speedup
██████████████████████████████████
This allows for the surface acquired from the pattern to have the
same content. In particular, in a case such as cairo_paint_with_alpha
we can now acquire an A8 mask surface instead of an ARGB32 mask
surface which can be rendered much more efficiently. This results
in a 4x speedup when using the OVER operator with the recently
added paint-with-alpha test:
Speedups
========
image-rgb paint-with-alpha_image_rgb_over-256 2.25 -> 0.60: 4.45x speedup
███▌
It does slowdown the same test when using the SOURCE operator, but
I don't think we care. Performing SOURCE with a mask is already a very
slow operation, (hitting compositeGeneral), so the slowdown here is
likely from having to convert from A8 back to ARGB32 before the
generalized compositing. So if someone cares about this slowdown,
(though SOURCE with cairo_paint_with_alpha doesn't seem extremely
useful), they will probably be motivated enough to contribute a
customized compositing function to replace compositeGeneral in which
case this slowdown should go away:
image-rgba paint-with-alpha_image_rgb_source-256 3.84 -> 8.86%: 1.94x slowdown
█
Most of the time pixman_region_init is called without any extents, and
followed by a pixman_region_union_rect, used to used to initialize
rectangular regions. pixman_region_union_rect is not that cheap, but
the sequence is called quite often. So it should be worth introducing
a specialized and fast function for this sequence.
This introduces pixman_region_init_rect. This new function makes
_cairo_region_init_from_rectangle obsolete.
Also removes the extent argument from pixman_region_init as it was
called with NULL most of the time. A pixman_region_init_with_extents
is added for the general case.
This is necessary to avoid many portability problems as cairoint.h includes
config.h. Without a test, we will regress again, hence add it.
The inclusion idiom for cairo now is:
#include "cairoint.h"
#include "cairo-something.h"
#include "cairo-anotherthing-private.h"
#include <some-library.h>
#include <other-library/other-file.h>
Moreover, some standard headers files are included from cairoint.h and need
not be included again.
This is a fix for a huge performance bug (as measured by perf/long-lines).
Previously, if no explicit clip was set, _clip_and_composite_trapezoids
would allocate a mask as large as the trapezoids and rasterize into it.
With this fix, it restricts the mask by the extents of the destination
surface.
This doesn't address the identical performance problem with the xlib
backend, which is due to a very similar bug in the X server.
image-rgb long-lines-uncropped-100 465.42 -> 5.03: 92.66x speedup
█████████████████████████████████████████████▉
image-rgba long-lines-uncropped-100 460.80 -> 5.02: 91.87x speedup
█████████████████████████████████████████████▍
The rule is: cairo_glyph_t* is always passed as const for measurement
purposes. This was not reflected in our public api previously. Fixed
Showing glyphs used to have cairo_glyph_t* always as const. With this
changed, it is only const on cairo_t and cairo_gstate_t operations.
cairo_surface_t, cairo_scaled_font_t, and individual backends receive
cairo_glyph_t* as non-const. The desired semantics is that they may modify
the contents of the array as long as they do not return
CAIRO_STATUS_UNSUPPORTED. This makes it possible to avoid copying the glyph
array again and again, and edit it in-place. Backends are in fact free to use
the array as a generic buffer as they see fit.
This is an attempt to fix the following bug:
http://bugzilla.gnome.org/show_bug.cgi?id=332266
With the recent rewrite of the device-offset code, which pushed things
from the gstate to the surface layer, the 16-bit limitations on coordinates
which previously applied to device space only, have lately been applying to
user space. This commit moves the device_transform back up above the conversion
from floating-point to fixed-point values so that once again the limitation
only applies to device space.
This is a step toward allowing device scaling in addition to device offsets.
So far, the scale values are still always 1.0 so only the translation is
actually being used. But most of the code is in place for doing scaling as
well and it just needs to be hooked up.
There are some fragile parts in this code, all of which involve using the
translation without the scale, (so grep for device_transform.x0 or
device_transform->x0). Some of these are likely bugs that will hopefully
be obvious once we start using the scale. Others are OK if only because
we 'know' that we aren't ever setting device scaling on a surface that
has a device offset (we only set device scaling on surfaces we create
internally and we don't export device scaling to the user).
All of these fragile parts in the code have been marked with comments of
the form: XXX: FRAGILE.
This rectangle has regular integer values, not fixed-point values.
So the old name was horribly wrong and misleading, (and yes I think
it was even I that had suggested it).
This patch was produced with the following (GNU) sed script:
sed -i -r -e 's/[ \t]+$//'
run on all *.[ch] files within cairo.
Note that the above script would have also created all the changes
from the previous commits to remove trailing whitespace.
This patch was produced with the following (GNU) sed script:
sed -i -r -e '/^[ \t]*\/?\*/ s/[ \t]+$//'
run on all *.[ch] files within cairo, (though I manually excluded
src/cairo-atsui-font.c which has a code line that appears as a comment
to this script).
This is a mega-patch that has the advantage that the entire test suite
passes both immediately before and immediately after this commit.
The disadvantage of the mega-patch is that it does not reflect the
development history of the device-offset branch, (with its various
fumblings and flailings). To capture that history, we will next merge
in that branch.
