Contents of /trunk/xorg-server/patches/xorg-server-1.16.1-glamor-upstream-fix.patch
Parent Directory | Revision Log
Revision 2512 -
(show annotations)
(download)
Wed Oct 29 15:02:06 2014 UTC (9 years, 11 months ago) by niro
File size: 8097 byte(s)
Wed Oct 29 15:02:06 2014 UTC (9 years, 11 months ago) by niro
File size: 8097 byte(s)
-upstream patches to glamor
1 | From 3c0431b8911241552a15a43e4279c50658b50a18 Mon Sep 17 00:00:00 2001 |
2 | From: Keith Packard <keithp@keithp.com> |
3 | Date: Wed, 16 Jul 2014 23:03:23 +0000 |
4 | Subject: glamor: Fix temp picture coordinates in glamor_composite_clipped_region |
5 | |
6 | To understand this patch, let's start at the protocol interface where |
7 | the relationship between the coordinate spaces is documented: |
8 | |
9 | static Bool |
10 | _glamor_composite(CARD8 op, |
11 | PicturePtr source, |
12 | PicturePtr mask, |
13 | PicturePtr dest, |
14 | INT16 x_source, |
15 | INT16 y_source, |
16 | INT16 x_mask, |
17 | INT16 y_mask, |
18 | INT16 x_dest, INT16 y_dest, |
19 | CARD16 width, CARD16 height, Bool fallback) |
20 | |
21 | The coordinates are passed to this function directly off the wire and |
22 | are all relative to their respective drawables. For Windows, this means |
23 | that they are relative to the upper left corner of the window, in |
24 | whatever pixmap that window is getting drawn to. |
25 | |
26 | _glamor_composite calls miComputeCompositeRegion to construct a clipped |
27 | region to actually render to. In reality, miComputeCompositeRegion clips |
28 | only to the destination these days; source clip region based clipping |
29 | would have to respect the transform, which isn't really possible. The |
30 | returned region is relative to the screen in which dest lives; offset by |
31 | dest->drawable.x and dest->drawable.y. |
32 | |
33 | What is important to realize here is that, because of clipping, the |
34 | composite region may not have the same position within the destination |
35 | drawable as x_dest, y_dest. The protocol coordinates now exist solely to |
36 | 'pin' the three objects together. |
37 | |
38 | extents->x1,y1 Screen origin of clipped operation |
39 | width,height Extents of the clipped operation |
40 | x_dest,y_dest Unclipped destination-relative operation coordinate |
41 | x_source,y_source Unclipped source-relative operation coordinate |
42 | x_mask,y_mask Unclipped mask-relative operation coordinate |
43 | |
44 | One thing we want to know is what the offset is from the original |
45 | operation origin to the clipped origin |
46 | |
47 | Destination drawable relative coordinates of the clipped operation: |
48 | |
49 | x_dest_clipped = extents->x1 - dest->drawable.x |
50 | y_dest_clipped = extents->y1 - dest->drawable.y |
51 | |
52 | Offset from the original operation origin: |
53 | |
54 | x_off_clipped = x_dest_clipped - x_dest |
55 | y_off_clipped = y_dest_clipped - y_dest |
56 | |
57 | Source drawable relative coordinates of the clipped operation: |
58 | |
59 | x_source_clipped = x_source + x_off_clipped; |
60 | y_source_clipped = y_source + y_off_clipped; |
61 | |
62 | Mask drawable relative coordinates of the clipped operation: |
63 | |
64 | x_mask_clipped = x_source + x_off_clipped; |
65 | y_mask_clipped = y_source + y_off_clipped; |
66 | |
67 | This is where the original code fails -- it doesn't subtract the |
68 | destination drawable location when computing the distance that the |
69 | operation has been moved by clipping. Here's what it does when |
70 | constructing a temporary source picture: |
71 | |
72 | temp_src = |
73 | glamor_convert_gradient_picture(screen, source, |
74 | extent->x1 + x_source - x_dest, |
75 | extent->y1 + y_source - y_dest, |
76 | width, height); |
77 | ... |
78 | x_temp_src = -extent->x1 + x_dest; |
79 | y_temp_src = -extent->y1 + y_dest; |
80 | |
81 | glamor_convert_gradient_picture needs source drawable relative |
82 | coordinates, but that is not what it's getting; it's getting |
83 | screen-relative coordinates for the destination, adjusted by the |
84 | distance between the provided source and destination operation |
85 | coordinates. We want x_source_clipped and y_source_clipped: |
86 | |
87 | x_source_clipped = x_source + x_off_clipped |
88 | = x_source + x_dest_clipped - x_dest |
89 | = x_source + extents->x1 - dest->drawable.x - x_dest |
90 | |
91 | x_temp_src/y_temp_src are supposed to be the coordinates of the original |
92 | operation translated to the temporary picture: |
93 | |
94 | x_temp_src = x_source - x_source_clipped; |
95 | y_temp_src = y_source - y_source_clipped; |
96 | |
97 | Note that x_source_clipped/y_source_clipped will never be less than |
98 | x_source/y_source because all we're doing is clipping. This means that |
99 | x_temp_src/y_temp_src will always be non-positive; the original source |
100 | coordinate can never be strictly *inside* the temporary image or we |
101 | could have made the temporary image smaller. |
102 | |
103 | x_temp_src = x_source - x_source_clipped |
104 | = x_source - (x_source + x_off_clipped) |
105 | = -x_off_clipped |
106 | = x_dest - x_dest_clipped |
107 | = x_dest - (extents->x1 - dest->drawable.x) |
108 | |
109 | Again, this is off by the destination origin within the screen |
110 | coordinate space. |
111 | |
112 | The code should look like: |
113 | |
114 | temp_src = |
115 | glamor_convert_gradient_picture(screen, source, |
116 | extent->x1 + x_source - x_dest - dest->pDrawable->x, |
117 | extent->y1 + y_source - y_dest - dest->pDrawable->y, |
118 | width, height); |
119 | |
120 | x_temp_src = -extent->x1 + x_dest + dest->pDrawable->x; |
121 | y_temp_src = -extent->y1 + y_dest + dest->pDrawable->y; |
122 | |
123 | Signed-off-by: Keith Packard <keithp@keithp.com> |
124 | Reviewed-by: Markus Wick <markus@selfnet.de> |
125 | (cherry picked from commit 55f5bfb578e934319d1308cbb56c900c5ac7cfa7) |
126 | Signed-off-by: Julien Cristau <jcristau@debian.org> |
127 | --- |
128 | diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c |
129 | index 14ab738..e5d5d2c 100644 |
130 | --- a/glamor/glamor_render.c |
131 | +++ b/glamor/glamor_render.c |
132 | @@ -1450,8 +1450,8 @@ glamor_composite_clipped_region(CARD8 op, |
133 | || source_pixmap->drawable.height != height)))) { |
134 | temp_src = |
135 | glamor_convert_gradient_picture(screen, source, |
136 | - extent->x1 + x_source - x_dest, |
137 | - extent->y1 + y_source - y_dest, |
138 | + extent->x1 + x_source - x_dest - dest->pDrawable->x, |
139 | + extent->y1 + y_source - y_dest - dest->pDrawable->y, |
140 | width, height); |
141 | if (!temp_src) { |
142 | temp_src = source; |
143 | @@ -1459,8 +1459,8 @@ glamor_composite_clipped_region(CARD8 op, |
144 | } |
145 | temp_src_priv = |
146 | glamor_get_pixmap_private((PixmapPtr) (temp_src->pDrawable)); |
147 | - x_temp_src = -extent->x1 + x_dest; |
148 | - y_temp_src = -extent->y1 + y_dest; |
149 | + x_temp_src = -extent->x1 + x_dest + dest->pDrawable->x; |
150 | + y_temp_src = -extent->y1 + y_dest + dest->pDrawable->y; |
151 | } |
152 | |
153 | if (mask |
154 | @@ -1474,8 +1474,8 @@ glamor_composite_clipped_region(CARD8 op, |
155 | * to do reduce one convertion. */ |
156 | temp_mask = |
157 | glamor_convert_gradient_picture(screen, mask, |
158 | - extent->x1 + x_mask - x_dest, |
159 | - extent->y1 + y_mask - y_dest, |
160 | + extent->x1 + x_mask - x_dest - dest->pDrawable->x, |
161 | + extent->y1 + y_mask - y_dest - dest->pDrawable->y, |
162 | width, height); |
163 | if (!temp_mask) { |
164 | temp_mask = mask; |
165 | @@ -1483,8 +1483,8 @@ glamor_composite_clipped_region(CARD8 op, |
166 | } |
167 | temp_mask_priv = |
168 | glamor_get_pixmap_private((PixmapPtr) (temp_mask->pDrawable)); |
169 | - x_temp_mask = -extent->x1 + x_dest; |
170 | - y_temp_mask = -extent->y1 + y_dest; |
171 | + x_temp_mask = -extent->x1 + x_dest + dest->pDrawable->x; |
172 | + y_temp_mask = -extent->y1 + y_dest + dest->pDrawable->y; |
173 | } |
174 | /* Do two-pass PictOpOver componentAlpha, until we enable |
175 | * dual source color blending. |
176 | -- |
177 | cgit v0.9.0.2-2-gbebe |
178 |