[cairo-bugs] [Bug 43397] EXTEND_NONE is used instead of EXTEND_PAD when src sample area is entirely within the surface

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=43397

--- Comment #29 from Andrea Canciani <ranma42 at gmail.com> 2012-01-21 03:21:56 PST ---
(In reply to comment #28)
> So, just to make sure I understand:

I'm afraid you missed a couple of points.

> Cairo is explicitly disobeying the call's EXTEND_PAD (as per Comment 15) and
> replaces it with EXTEND_NONE in this piece of code:

Cairo is not disobeying anything. It accepted a CAIRO_EXTEND_PAD and decided to
internally replace with CAIRO_EXTEND_NONE, because they are equivalent and
CAIRO_EXTEND_NONE has some desirable properties.
Please notice that this change is *NOT* observable from outside Cairo in any
way.
Cairo could have legitimately replaced it with CAIRO_PRIVATE_EXTEND_ANY and
everything would still be ok from the Cairo point of view (i.e. the result is
correct, the backends have the information needed to perform any appropriate
optimization).
It happens that (the non-existing) CAIRO_PRIVATE_EXTEND_ANY and
CAIRO_EXTEND_NONE both map to RepeatNone in the xlib backend.
In the first case this happens because, since any of
Repeat{None,Pad,Reflect,Normal} would produce the same result, Cairo goes for
the default one, hoping/expecting that defaults are optimized as much as
possible in the underlying layers.
In the second case Cairo *must* use RepeatNone, because it believes that
otherwise the result will be different.

> 
> if (attr->extend != CAIRO_EXTEND_REPEAT) {
>  ...
> } else {
>  if (sampled_area.x >= extents.x &&
>  sampled_area.y >= extents.y &&
>  sampled_area.x + (int) sampled_area.width <= extents.x + (int) extents.width
> &&
>  sampled_area.y + (int) sampled_area.height <= extents.y + (int)
> extents.height)
>  {
>  /* source is wholly contained within extents, drop the REPEAT */
>  extents = sampled_area;
>  attr->extend = CAIRO_EXTEND_NONE;
>  }
> ...
> 
> 
> Wouldn't it make more sense to simply check for EXTEND_PAD in this code and
> skip conversion to EXTEND_NONE?

No, that is not the issue. Cairo knows that EXTEND_PAD, EXTEND_NONE, anything
have the same result. It is just guessing that the default will be fast.
Unfortunately this is not the case for that specific driver, but might very
well be in many other cases.
Can you guarantee that no driver will have regressions if it gets PAD instead
of NONE?
Optimizing the default case looks quite important, so I would definitely try to
do that, instead of trying to avoid the default.

> Or maybe check whether or not XRENDER is being used and if not, don't touch the extend flag?

In this case XRENDER is being used, so I guess you would have to suggest the
opposite to change Cairo behavior, i.e.:
Or maybe check whether or not XRENDER is being used and if *it is*, don't touch
the extend flag?

I believe that most of this misunderstanding is due to the similarity in names
(and ease of mapping) between Cairo extend modes and Xlib repeat modes.
Please remember that they are not the same thing.
(This is not that crazy, it might really happen. Some time ago I remember that
there was some discussion about allowing different vertical/horizontal extend
modes)

Shall I ping the xorg-devel list/bug 35579/anybody asking if detecting the
"ANY" extend mode in X is acceptable?

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.