Re: [PATCH xserver 1/3] glamor: Scissor rectangle drawing to the bounds of the rects.

2017-08-03 Thread Keith Packard 
Eric Anholt  writes:

> Scissors provide a critical hint to tiled renderers as to what tiles
> need to be load/stored because they could be modified by the
> rendering.
>
> The bounds calculation here is limited to when we have a small number
> of rects (large enough to cover rounded window corners, but probably
> not xeyes) to avoid overhead on desktop GL.
>
> No performance difference on i965 with x11perf -rect1 -repeat 1 -reps
> 1 (n=50)
>
> v2: Clamp rectangle bounds addition.

For the series:

Reviewed-by: Keith Packard 

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/3] glamor: Scissor rectangle drawing to the bounds of the rects.

2017-08-03 Thread Eric Anholt 
Scissors provide a critical hint to tiled renderers as to what tiles
need to be load/stored because they could be modified by the
rendering.

The bounds calculation here is limited to when we have a small number
of rects (large enough to cover rounded window corners, but probably
not xeyes) to avoid overhead on desktop GL.

No performance difference on i965 with x11perf -rect1 -repeat 1 -reps
1 (n=50)

v2: Clamp rectangle bounds addition.

Signed-off-by: Eric Anholt 
---
 glamor/glamor_rects.c | 26 ++
 glamor/glamor_utils.h | 35 +++
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c
index cc029c8c04a6..6cbb040c18ea 100644
--- a/glamor/glamor_rects.c
+++ b/glamor/glamor_rects.c
@@ -53,6 +53,7 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
 char *vbo_offset;
 int box_index;
 Bool ret = FALSE;
+BoxRec bounds = glamor_no_rendering_bounds();
 
 pixmap_priv = glamor_get_pixmap_private(pixmap);
 if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
@@ -60,6 +61,12 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
 
 glamor_make_current(glamor_priv);
 
+if (nrect < 100) {
+bounds = glamor_start_rendering_bounds();
+for (int i = 0; i < nrect; i++)
+glamor_bounds_union_rect(, [i]);
+}
+
 if (glamor_priv->glsl_version >= 130) {
 prog = glamor_use_program_fill(pixmap, gc,
_priv->poly_fill_rect_program,
@@ -121,11 +128,22 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
 goto bail;
 
 while (nbox--) {
-glScissor(box->x1 + off_x,
-  box->y1 + off_y,
-  box->x2 - box->x1,
-  box->y2 - box->y1);
+BoxRec scissor = {
+.x1 = max(box->x1, bounds.x1 + drawable->x),
+.y1 = max(box->y1, bounds.y1 + drawable->y),
+.x2 = min(box->x2, bounds.x2 + drawable->x),
+.y2 = min(box->y2, bounds.y2 + drawable->y),
+};
+
 box++;
+
+if (scissor.x1 >= scissor.x2 || scissor.y1 >= scissor.y2)
+continue;
+
+glScissor(scissor.x1 + off_x,
+  scissor.y1 + off_y,
+  scissor.x2 - scissor.x1,
+  scissor.y2 - scissor.y1);
 if (glamor_priv->glsl_version >= 130)
 glDrawArraysInstanced(GL_TRIANGLE_STRIP, 0, 4, nrect);
 else {
diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h
index a35917c37a16..f1f8f5633a1a 100644
--- a/glamor/glamor_utils.h
+++ b/glamor/glamor_utils.h
@@ -729,6 +729,41 @@ glamor_make_current(glamor_screen_private *glamor_priv)
 }
 }
 
+static inline BoxRec
+glamor_no_rendering_bounds(void)
+{
+BoxRec bounds = {
+.x1 = 0,
+.y1 = 0,
+.x2 = MAXSHORT,
+.y2 = MAXSHORT,
+};
+
+return bounds;
+}
+
+static inline BoxRec
+glamor_start_rendering_bounds(void)
+{
+BoxRec bounds = {
+.x1 = MAXSHORT,
+.y1 = MAXSHORT,
+.x2 = 0,
+.y2 = 0,
+};
+
+return bounds;
+}
+
+static inline void
+glamor_bounds_union_rect(BoxPtr bounds, xRectangle *rect)
+{
+bounds->x1 = min(bounds->x1, rect->x);
+bounds->y1 = min(bounds->y1, rect->y);
+bounds->x2 = min(SHRT_MAX, max(bounds->x2, rect->x + rect->width));
+bounds->y2 = min(SHRT_MAX, max(bounds->y2, rect->y + rect->height));
+}
+
 /**
  * Helper function for implementing draws with GL_QUADS on GLES2,
  * where we don't have them.
-- 
2.13.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/3] glamor: Scissor rectangle drawing to the bounds of the rects.

2017-08-02 Thread Eric Anholt 
Michel Dänzer  writes:

> On 02/08/17 05:59 AM, Eric Anholt wrote:
>> Scissors provide a critical hint to tiled renderers as to what tiles
>> need to be load/stored because they could be modified by the
>> rendering.
>> 
>> The bounds calculation here is limited to when we have a small number
>> of rects (large enough to cover rounded window corners, but probably
>> not xeyes) to avoid overhead on desktop GL.
>> 
>> No performance difference on i965 with x11perf -rect1 -repeat 1 -reps
>> 1 (n=50)
>> 
>> Signed-off-by: Eric Anholt 
>> ---
>>  glamor/glamor_rects.c | 26 ++
>>  glamor/glamor_utils.h | 35 +++
>>  2 files changed, 57 insertions(+), 4 deletions(-)
>> 
>> diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c
>> index cc029c8c04a6..6cbb040c18ea 100644
>> --- a/glamor/glamor_rects.c
>> +++ b/glamor/glamor_rects.c
>> @@ -53,6 +53,7 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
>>  char *vbo_offset;
>>  int box_index;
>>  Bool ret = FALSE;
>> +BoxRec bounds = glamor_no_rendering_bounds();
>>  
>>  pixmap_priv = glamor_get_pixmap_private(pixmap);
>>  if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
>> @@ -60,6 +61,12 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
>>  
>>  glamor_make_current(glamor_priv);
>>  
>> +if (nrect < 100) {
>> +bounds = glamor_start_rendering_bounds();
>> +for (int i = 0; i < nrect; i++)
>> +glamor_bounds_union_rect(, [i]);
>> +}
>
> Did your testing hit the nrect == 99 cases? I'd expect those to have the
> most potential impact on throughput.

No, I didn't.  However, the other two patches were nrect=1 cases in
x11perf, which I think would be even more impacted than nrect=99, and
they showed improvements.


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/3] glamor: Scissor rectangle drawing to the bounds of the rects.

2017-08-01 Thread Michel Dänzer 
On 02/08/17 05:59 AM, Eric Anholt wrote:
> Scissors provide a critical hint to tiled renderers as to what tiles
> need to be load/stored because they could be modified by the
> rendering.
> 
> The bounds calculation here is limited to when we have a small number
> of rects (large enough to cover rounded window corners, but probably
> not xeyes) to avoid overhead on desktop GL.
> 
> No performance difference on i965 with x11perf -rect1 -repeat 1 -reps
> 1 (n=50)
> 
> Signed-off-by: Eric Anholt 
> ---
>  glamor/glamor_rects.c | 26 ++
>  glamor/glamor_utils.h | 35 +++
>  2 files changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c
> index cc029c8c04a6..6cbb040c18ea 100644
> --- a/glamor/glamor_rects.c
> +++ b/glamor/glamor_rects.c
> @@ -53,6 +53,7 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
>  char *vbo_offset;
>  int box_index;
>  Bool ret = FALSE;
> +BoxRec bounds = glamor_no_rendering_bounds();
>  
>  pixmap_priv = glamor_get_pixmap_private(pixmap);
>  if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
> @@ -60,6 +61,12 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
>  
>  glamor_make_current(glamor_priv);
>  
> +if (nrect < 100) {
> +bounds = glamor_start_rendering_bounds();
> +for (int i = 0; i < nrect; i++)
> +glamor_bounds_union_rect(, [i]);
> +}

Did your testing hit the nrect == 99 cases? I'd expect those to have the
most potential impact on throughput.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 1/3] glamor: Scissor rectangle drawing to the bounds of the rects.

2017-08-01 Thread Keith Packard 
Eric Anholt  writes:

> Scissors provide a critical hint to tiled renderers as to what tiles
> need to be load/stored because they could be modified by the
> rendering.
>
> The bounds calculation here is limited to when we have a small number
> of rects (large enough to cover rounded window corners, but probably
> not xeyes) to avoid overhead on desktop GL.

Hrm. Often times, the rendering comes from a region which already has a
bounds computed. I wonder if it would be useful to somehow wrap
PaintWindow and capture the bounding rect from there?

+static inline void
+glamor_bounds_union_rect(BoxPtr bounds, xRectangle *rect)
+{
+bounds->x1 = min(bounds->x1, rect->x);
+bounds->y1 = min(bounds->y1, rect->y);
+bounds->x2 = max(bounds->x2, rect->x + rect->width);
+bounds->y2 = max(bounds->y2, rect->y + rect->height);
+}

You're in a world of pain here -- rect->width is an unsigned 16-bit
value, bounds->x2 and rect->x are signed 16-bit values.

Maybe something like:

bounds->x2 = min(SHRT_MAX, max(bounds->x2, rect->x + rect->width));
bounds->y2 = min(SHRT_MAX, max(bounds->y2, rect->y + rect->height));

?

-- 
-keith


signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 1/3] glamor: Scissor rectangle drawing to the bounds of the rects.

2017-08-01 Thread Eric Anholt 
Scissors provide a critical hint to tiled renderers as to what tiles
need to be load/stored because they could be modified by the
rendering.

The bounds calculation here is limited to when we have a small number
of rects (large enough to cover rounded window corners, but probably
not xeyes) to avoid overhead on desktop GL.

No performance difference on i965 with x11perf -rect1 -repeat 1 -reps
1 (n=50)

Signed-off-by: Eric Anholt 
---
 glamor/glamor_rects.c | 26 ++
 glamor/glamor_utils.h | 35 +++
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c
index cc029c8c04a6..6cbb040c18ea 100644
--- a/glamor/glamor_rects.c
+++ b/glamor/glamor_rects.c
@@ -53,6 +53,7 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
 char *vbo_offset;
 int box_index;
 Bool ret = FALSE;
+BoxRec bounds = glamor_no_rendering_bounds();
 
 pixmap_priv = glamor_get_pixmap_private(pixmap);
 if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
@@ -60,6 +61,12 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
 
 glamor_make_current(glamor_priv);
 
+if (nrect < 100) {
+bounds = glamor_start_rendering_bounds();
+for (int i = 0; i < nrect; i++)
+glamor_bounds_union_rect(, [i]);
+}
+
 if (glamor_priv->glsl_version >= 130) {
 prog = glamor_use_program_fill(pixmap, gc,
_priv->poly_fill_rect_program,
@@ -121,11 +128,22 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable,
 goto bail;
 
 while (nbox--) {
-glScissor(box->x1 + off_x,
-  box->y1 + off_y,
-  box->x2 - box->x1,
-  box->y2 - box->y1);
+BoxRec scissor = {
+.x1 = max(box->x1, bounds.x1 + drawable->x),
+.y1 = max(box->y1, bounds.y1 + drawable->y),
+.x2 = min(box->x2, bounds.x2 + drawable->x),
+.y2 = min(box->y2, bounds.y2 + drawable->y),
+};
+
 box++;
+
+if (scissor.x1 >= scissor.x2 || scissor.y1 >= scissor.y2)
+continue;
+
+glScissor(scissor.x1 + off_x,
+  scissor.y1 + off_y,
+  scissor.x2 - scissor.x1,
+  scissor.y2 - scissor.y1);
 if (glamor_priv->glsl_version >= 130)
 glDrawArraysInstanced(GL_TRIANGLE_STRIP, 0, 4, nrect);
 else {
diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h
index a35917c37a16..824be1a09338 100644
--- a/glamor/glamor_utils.h
+++ b/glamor/glamor_utils.h
@@ -729,6 +729,41 @@ glamor_make_current(glamor_screen_private *glamor_priv)
 }
 }
 
+static inline BoxRec
+glamor_no_rendering_bounds(void)
+{
+BoxRec bounds = {
+.x1 = 0,
+.y1 = 0,
+.x2 = MAXSHORT,
+.y2 = MAXSHORT,
+};
+
+return bounds;
+}
+
+static inline BoxRec
+glamor_start_rendering_bounds(void)
+{
+BoxRec bounds = {
+.x1 = MAXSHORT,
+.y1 = MAXSHORT,
+.x2 = 0,
+.y2 = 0,
+};
+
+return bounds;
+}
+
+static inline void
+glamor_bounds_union_rect(BoxPtr bounds, xRectangle *rect)
+{
+bounds->x1 = min(bounds->x1, rect->x);
+bounds->y1 = min(bounds->y1, rect->y);
+bounds->x2 = max(bounds->x2, rect->x + rect->width);
+bounds->y2 = max(bounds->y2, rect->y + rect->height);
+}
+
 /**
  * Helper function for implementing draws with GL_QUADS on GLES2,
  * where we don't have them.
-- 
2.13.3

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel