Annotation of /trunk/xorg-server/patches/xorg-server-1.16.1-glamor-upstream-fix.patch
Parent Directory | Revision Log
Revision 2512 -
(hide 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 | niro | 2512 | 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 |