Re: [PATCH xserver] glamor: Disable logic ops when doing compositing

[Markus Wick]

Hi,

AFAIK we disable logic ops after each usage, so this seems a bit 
redundant to me. Did we miss one usages? As logic ops aren't very 
common, this sounds good to me.


Am 2016-05-13 13:29, schrieb Keith Packard:

If the logic op gets left enabled, it overrides the blending
operation, causing incorrect contents on the display.

Signed-off-by: Keith Packard 
---
 glamor/glamor_program.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/glamor/glamor_program.c b/glamor/glamor_program.c
index 0a94de6..a43ee6e 100644
--- a/glamor/glamor_program.c
+++ b/glamor/glamor_program.c
@@ -499,6 +499,7 @@ glamor_set_blend(CARD8 op, glamor_program_alpha
alpha, PicturePtr dst)
 }

 glEnable(GL_BLEND);
+glDisable(GL_COLOR_LOGIC_OP);
 glBlendFunc(src_blend, dst_blend);
 }


___
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: [PULL] glamor-next

[Markus Wick]

Am 2015-03-26 03:33, schrieb Eric Anholt:

The only reason _nf is useful is if you need to implement faster
software fallbacks -- otherwise, you should wrap above glamor, since 
_nf

would have been equivalent to calling down.


For faster fallbacks, we could delay the uploading part at the end of 
the fallback. So if the next call is also a fallback, we'd not have to 
readback it again.
But this is in opposition to only readback the affected part of the 
pixmap.

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

[PATCH] xwayland: Set glamor filter to nearest

[Markus Wick]

glEGLImageTargetTexture2DOES only set the first level.
Mesa seems to handle this new texture as incomplete and renders a black screen.

In my opinion, this call shall never return an incomplete texture, but this
mesa issue is easy to work around. We also want to prevent linear filtering.

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

Signed-off-by: Markus Wick mar...@selfnet.de
---
 hw/xwayland/xwayland-glamor.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index 09b454f..dd85518 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -137,6 +137,9 @@ xwl_glamor_create_pixmap_for_bo(ScreenPtr screen, struct 
gbm_bo *bo, int depth)
 
 glGenTextures(1, xwl_pixmap-texture);
 glBindTexture(GL_TEXTURE_2D, xwl_pixmap-texture);
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
 glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, xwl_pixmap-image);
 glBindTexture(GL_TEXTURE_2D, 0);
 
-- 
2.2.2

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

[PATCH] xwayland: Set glamor filter to nearest

[Markus Wick]

glEGLImageTargetTexture2DOES only set the first level.
Mesa handles this new texture as incomplete and renders a black screen.
We also want to prevent linear filtering.

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

Signed-off-by: Markus Wick mar...@selfnet.de
---
 hw/xwayland/xwayland-glamor.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c
index 09b454f..dd85518 100644
--- a/hw/xwayland/xwayland-glamor.c
+++ b/hw/xwayland/xwayland-glamor.c
@@ -137,6 +137,9 @@ xwl_glamor_create_pixmap_for_bo(ScreenPtr screen, struct 
gbm_bo *bo, int depth)
 
 glGenTextures(1, xwl_pixmap-texture);
 glBindTexture(GL_TEXTURE_2D, xwl_pixmap-texture);
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
 glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, xwl_pixmap-image);
 glBindTexture(GL_TEXTURE_2D, 0);
 
-- 
2.2.2

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

Re: 1.17 status and schedule

[Markus Wick]

Yeah, I did wrote it. But meanwhile, I'd remove the MAX_LEVEL parameter 
as this isn't allowed in gl es.


@Jasper: Texture storage isn't possible here as this texture is 
allocated by egl.

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

Re: [PATCH] glamor: Use GL_STREAM_READ also for read/write access to a PBO

[Markus Wick]

Am 2014-09-25 08:27, schrieb Michel Dänzer:

From: Michel Dänzer michel.daen...@amd.com

Otherwise the CPU may end up reading from non-cacheable memory, which 
is

very slow.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84178
Signed-off-by: Michel Dänzer michel.daen...@amd.com
---

Keeping gl_usage in case we need to add back GLAMOR_ACCESS_WO.

 glamor/glamor_prepare.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/glamor/glamor_prepare.c b/glamor/glamor_prepare.c
index 561c55d..fb85d90 100644
--- a/glamor/glamor_prepare.c
+++ b/glamor/glamor_prepare.c
@@ -84,10 +84,7 @@ glamor_prep_pixmap_box(PixmapPtr pixmap,
glamor_access_t access, BoxPtr box)
 if (priv-base.pbo == 0)
 glGenBuffers(1, priv-base.pbo);

-if (access == GLAMOR_ACCESS_RW)
-gl_usage = GL_DYNAMIC_DRAW;
-else
-gl_usage = GL_STREAM_READ;
+gl_usage = GL_STREAM_READ;

 glBindBuffer(GL_PIXEL_PACK_BUFFER, priv-base.pbo);
 glBufferData(GL_PIXEL_PACK_BUFFER,


Was the write only patch merged? If so, this should be changed to use 
GL_STREAM_DRAW for WO.


But you're right, for RW, it should be STREAM (used once) READ (accessed 
by the CPU), so


Reviewed-by: Markus Wick mar...@selfnet.de
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Revert glamor: Fix coordinates handling for composite source/mask pictures

[Markus Wick]

Hi,

kwin still works fine.

If you'll remove the ErrorF lines, this patch is:
Reviewed-by: Markus Wick mar...@selfnet.de

Am 2014-07-17 01:13, schrieb Keith Packard:

I think I understand the code and now believe that both the original
fixed versions are incorrect. However, I cannot reproduce any of the
problems we've had reported. I've placed an explanation of the bug and
the fix in the patch attached here. In theory, this should fix *both*
the original kwin problem and the systray icons. I would love to see
this tested soon if possible. I wouldn't mind slipping this into 1.16;
it seems low risk to me, and solves a fairly serious regression with
glamor.

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

Re: [PATCH] glamor: Fix stack corruption, glGet on GL_MAX_VIEWPORT_DIMS returns two values

[Markus Wick]

Oh, how did I miss this ...

Reviewed-by: Markus Wick mar...@selfnet.de

Am 2014-06-28 21:27, schrieb Tomasz Borowik:

From 0a6d1c900b6d5559c50c703480a479a59689b8be Mon Sep 17 00:00:00 2001
From: timon37 timo...@gmail.com
Date: Sat, 28 Jun 2014 21:26:27 +0200
Subject: [PATCH] glamor: Fix stack corruption in glamor_init, glGet on
 GL_MAX_VIEWPORT_DIMS returns two values

---
 glamor/glamor.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index c398807..3588903 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -316,7 +316,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 {
 glamor_screen_private *glamor_priv;
 int gl_version;
-int max_viewport_size;
+int max_viewport_size[2];

 #ifdef RENDER
 PictureScreenPtr ps = GetPictureScreenIfSet(screen);
@@ -408,8 +408,9 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 glamor_priv-has_buffer_storage =
 epoxy_has_gl_extension(GL_ARB_buffer_storage);
 glGetIntegerv(GL_MAX_TEXTURE_SIZE, glamor_priv-max_fbo_size);
-glGetIntegerv(GL_MAX_VIEWPORT_DIMS, max_viewport_size);
-glamor_priv-max_fbo_size = MIN(glamor_priv-max_fbo_size,
max_viewport_size);
+glGetIntegerv(GL_MAX_VIEWPORT_DIMS, max_viewport_size);
+glamor_priv-max_fbo_size = MIN(glamor_priv-max_fbo_size,
max_viewport_size[0]);
+glamor_priv-max_fbo_size = MIN(glamor_priv-max_fbo_size,
max_viewport_size[1]);
 #ifdef MAX_FBO_SIZE
 glamor_priv-max_fbo_size = MAX_FBO_SIZE;
 #endif

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

Re: [PATCH 04/13] glamor: Use glamor_program and GL_LINES for 0-width lines

[Markus Wick]

Hi Keith,

here a small summary of my monolog on #org-devel:

Am 2014-05-13 18:02, schrieb Keith Packard:
So in the end, I fear we have to add an offset based on the direction 
of

the line. But this shouldn't be as hard with instancing :)

No, we just need to move the lines so that they line up with the GL
pixel centers.


I don't think it's that easy. The required calculations are all done 
with floats, so they don't guarantee to hit the center of the pixel 
exactly. As a small offset from the center of the pixels affects the 
rasterization, this calculation isn't stable.



So neither the first nor the last pixel is defined to be
overwritten. I doubt X11 defines the same :/
I don't understand this -- GL defines the lines rasterization rule to 
be

identical to Bresenham's algorithm.


I have reread this part of the 4.4 specs for the thrid time now, but I'm 
still unsure what it guarantees.
In the beginning of 14.5.1, it's defined in the way you're using it. But 
in the second part of this section, they relax this specs again:



Because the initial and final conditions of the diamond-exit rule may be 
difficult
to implement, other line segment rasterization algorithms are allowed, 
subject to

the following rules:

[...]

4. If two line segments share a common endpoint, and both segments are 
either
   x-major (both left-to-right or both right-to-left) or y-major (both 
bottom-to-
   top or both top-to-bottom), then rasterizing both segments may not 
produce
   duplicate fragments, nor may any fragments be omitted so as to 
interrupt

   continuity of the connected segments.


And this relaxed version is incompatible with the former one, eg shown 
by a horizontal from A via B to C (all on the center of pixels):
The lines A-B and C-B are defined to overdraw B in the former case but 
defined to _not_ overdraw B in the second case.


However, I've created a test program for the rasterization rules:
http://pastie.org/9175737
i965 results: http://markus.members.selfnet.de/xorg/gl_lines.png
The result should be a gray surface.

I think we have to move the endpoints to get stable + defined 
rasterization results. But this is as easy with instancing or geometry 
shaders.


What do you want to do on devices which doesn't support any of this 
extensions? imo we could fall back to mi or transfer in software :/



I actually think that GL's semantics are pretty nice here; it defines
lines to *never* draw the last pixel. The application can explicitly
draw one if desired. This eliminates another piece of state without
reducing functionality, which is pretty sweet.


Yeah, this hack is really nice. As both instancing + geometry shaders 
could be implemented in the same way (draw the first but not the last 
pixel), we could still use it. But the code lacks a comment why we just 
add 1 to some coord.

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

[PATCH 2/3] glamor: Fix no-mipmap allocations

[Markus Wick]

With GL_TEXTURE_MIN_FILTER, we configure not to use mipmaps, but we don't 
configure to not have them.
Some driver allocate the complete texture + all mipmaps at once, so they will 
still allocate them.
GL_TEXTURE_MAX_LEVEL however disable the mipmaps at all, so eg nvidia stop 
spamming vram wasting warnings.

Signed-off-by: Markus Wick mar...@selfnet.de
---
 glamor/glamor_fbo.c| 1 +
 glamor/glamor_font.c   | 1 +
 glamor/glamor_pixmap.c | 1 +
 3 files changed, 3 insertions(+)

diff --git a/glamor/glamor_fbo.c b/glamor/glamor_fbo.c
index 5521683..090dfd8 100644
--- a/glamor/glamor_fbo.c
+++ b/glamor/glamor_fbo.c
@@ -347,6 +347,7 @@ _glamor_create_tex(glamor_screen_private *glamor_priv,
 glamor_make_current(glamor_priv);
 glGenTextures(1, tex);
 glBindTexture(GL_TEXTURE_2D, tex);
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0);
 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 glTexImage2D(GL_TEXTURE_2D, 0, format, w, h, 0,
diff --git a/glamor/glamor_font.c b/glamor/glamor_font.c
index f747d59..57c607d 100644
--- a/glamor/glamor_font.c
+++ b/glamor/glamor_font.c
@@ -95,6 +95,7 @@ glamor_font_get(ScreenPtr screen, FontPtr font)
 glActiveTexture(GL_TEXTURE0);
 glBindTexture(GL_TEXTURE_2D, glamor_font-texture_id);
 
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0);
 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 
diff --git a/glamor/glamor_pixmap.c b/glamor/glamor_pixmap.c
index 54b414b..789d377 100644
--- a/glamor/glamor_pixmap.c
+++ b/glamor/glamor_pixmap.c
@@ -717,6 +717,7 @@ __glamor_upload_pixmap_to_texture(PixmapPtr pixmap, 
unsigned int *tex,
 }
 
 glBindTexture(GL_TEXTURE_2D, *tex);
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0);
 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 glPixelStorei(GL_UNPACK_ALIGNMENT, 4);
-- 
1.9.2

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

[PATCH 1/3] glamor: Fix memory leak in put_image

[Markus Wick]

Signed-off-by: Markus Wick mar...@selfnet.de
---
 glamor/glamor_image.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/glamor/glamor_image.c b/glamor/glamor_image.c
index 4791d08..c087a13 100644
--- a/glamor/glamor_image.c
+++ b/glamor/glamor_image.c
@@ -92,6 +92,7 @@ glamor_put_image_bail(DrawablePtr drawable, GCPtr gc, int 
depth, int x, int y,
 glamor_prepare_access_gc(gc))
 fbPutImage(drawable, gc, depth, x, y, w, h, leftPad, format, bits);
 glamor_finish_access(drawable);
+glamor_finish_access_gc(gc);
 }
 
 void
-- 
1.9.2

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

[PATCH 3/3] glamor: Choose max fbo size by texture + viewport size

[Markus Wick]

The max size of renderbuffers and texture often match by accident, but as we 
always use textures, we should check for the right flag.
Also check for viewport size as this may be lower and we want to render to 
almost every pixmap.

Signed-off-by: Markus Wick mar...@selfnet.de
---
 glamor/glamor.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 08f6ba1..c398807 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -316,6 +316,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 {
 glamor_screen_private *glamor_priv;
 int gl_version;
+int max_viewport_size;
 
 #ifdef RENDER
 PictureScreenPtr ps = GetPictureScreenIfSet(screen);
@@ -406,7 +407,9 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 epoxy_has_gl_extension(GL_ARB_map_buffer_range);
 glamor_priv-has_buffer_storage =
 epoxy_has_gl_extension(GL_ARB_buffer_storage);
-glGetIntegerv(GL_MAX_RENDERBUFFER_SIZE, glamor_priv-max_fbo_size);
+glGetIntegerv(GL_MAX_TEXTURE_SIZE, glamor_priv-max_fbo_size);
+glGetIntegerv(GL_MAX_VIEWPORT_DIMS, max_viewport_size);
+glamor_priv-max_fbo_size = MIN(glamor_priv-max_fbo_size, 
max_viewport_size);
 #ifdef MAX_FBO_SIZE
 glamor_priv-max_fbo_size = MAX_FBO_SIZE;
 #endif
-- 
1.9.2

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

Re: [PATCH 13/20] glamor: Add glamor_program based copy acceleration

[Markus Wick]

Am 2014-03-19 06:09, schrieb Keith Packard:

+glamor_pixmap_loop(src_priv, src_box_x, src_box_y) {
+glamor_pixmap_loop(dst_priv, dst_box_x, dst_box_y) {
+for (b = 0; b  nbox; b++) {


Are you sure that this loop order is safe?
Eg what happens, when we copy the first box by box_size/2?
I think we will overwrite the second part of the first box at first and 
then try to copy the overwritten content into the second box.


To fix this issue, we have to loop over the boxes in opposition to the 
copy order. :/


What will happen if we have two boxes defined and the copy of the first 
box will overlapp the second box? Shall we copy the new or the old 
content of the second box? If it's defined to be the new content, we 
also have to move the box loop to the top.


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

Re: [PATCH 14/20] glamor: stub out lines

[Markus Wick]

Am 2014-03-24 20:47, schrieb Keith Packard:
Even still, uxa is significantly faster than glamor on this one; uxa 
has

a simple test for a request that contains only vertical and horizontal
lines and fills those as rectangles. We could easily do the same thing
in glamor, which should yield better performance for this case. In 
fact,

I suspect we could do the test for all vertical/horizontal with the CPU
and let a vertex shader compute rectangles from the existing line
coordinates. Might be a fun exercise.

1: intel-glamor-line.perf
2: intel-uxa.perf

   1 2 Operation
   -   -
   402.0 1070.0 ( 2.662)   500-pixel horizontal line 
segment
177000.0  986.0 (55.706)   500-pixel vertical line 
segment


But shouldn't UXA be affected as much as glamor by cache issues?
My test results on ivb glamor vs SNA looks funny as SNA seems to be as 
slow as glamor:


glamor:
300 reps @   0.0004 msec (235.0/sec): 500-pixel horizontal line 
segment
 60 reps @   0.0018 msec (565000.0/sec): 500-pixel vertical line 
segment


SNA:
800 reps @   0.0001 msec (729.0/sec): 500-pixel horizontal line 
segment
 40 reps @   0.0024 msec (413000.0/sec): 500-pixel vertical line 
segment


As UXA is so much faster, could it be because of line rasterization 
limits of the gpu itself?

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

Re: [PATCH 00/10] First Glamor core acceleration attempt

[Markus Wick]

Am 2014-03-21 00:46, schrieb Keith Packard:

Patches 7-9 are really nice, but there should be a glsl130+instancing
fallback with another vertex format + shaders.
I'd like to know which hardware is going to need this particular 
fallback?


eg ironlake. But I more think about new GPU where the driver just isn't 
in such a good state. It would be nice to use glamor on very new gpus 
without requiring a full features gl3.3 driver.



But I get some broken renderings in xterm on nvidia because of the 
last

patch. I guess it's because of the 0.5 offset.

Can you try my latest glamor-server branch and see if that has been
fixed? I added the conditional offset stuff as suggested by you and 
Eric

to that.


Yeah, it was fine. Sorry for not mention it.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 10/10] glamor: Add glamor_program based poly_text and image_text

[Markus Wick]

Am 2014-03-21 00:43, schrieb Keith Packard:

more important to only get one mipmap level: max_level = 0

I copied this code from elsewhere; do you have a substitute suggestion?


glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0);
The other parameters are sampler parameters, max_level is a texture 
parameter and affects allocation.



Is a font always realized? I wonder as we reallize on font_get.
realized just means that I've uploaded the glyph data to the GPU; 
it's
per-GPU, and hence per-screen to us. I delay until the first font_get 
in

case you've got multiple screens and use a font on only a subset.


So what will happen if a program only alloc and delete a font? As 
font_get wasn't called, we delete a not initialized font.

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

Re: [PATCH 10/20] glamor: Add glamor_program based fill/set/get spans

[Markus Wick]

Am 2014-03-20 22:55, schrieb Keith Packard:

tbh, without an PBO, this is wrose than the fb fallback as it's one
stall per span.


Yes, it's completely miserable. However, it will never be called as the
only paths to get here are short-circuited above this in the driver. 
The

only way to even test this code is to disable much of the remaining
acceleration code.

Also with PBO, it's still a huge overhead per span. I guess it's 
faster

for lots of spans (maybe 20+) to always fall back to fb.
But I don't see another non-hacky way to implement this.


And, as it won't be executed in the future, we'd best leave it as 
simple

as possible. Bits of the X 2D API are 'sub-optimally designed' :-)


The simplest way is to always fall back to fb. So I think we shouldn't 
implement get_spans at all but fb.

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

Re: [PATCH 13/20] glamor: Add glamor_program based copy acceleration

[Markus Wick]

Am 2014-03-20 23:07, schrieb Keith Packard:

+glCopyPixels (dx1 + dx - src_box-x1,
+  dy1 + dy - src_box-y1,
+  dx2 - dx1, dy2 - dy1, GL_COLOR);


This functions are deprecated fortunately. Please don't use them at 
all.

Glamor shall be used as hardware accelerated fallback, so we shouldn't
rely on uncommon features which may be implemented in software (eg on
nvidia) or not implemented at all (core, gles).


There isn't anything else I *can* use. GL doesn't otherwise define the
results for operations using the same object for source and
destination.


There isn't anything you can use. There is no point in using the 
deprecated function in an undefined way.

No GL copy function is defined for overlapping rects.
btw, you only check if the src and the dst are the same, you don't check 
for overlapping src+dst.




I guess GPUs have some special HW for this, but we can't use them on
OpenGL. memmove isn't parallelizable.


Yes, Intel GPUs have the 'blt' engine which performs overlapped copies
correctly, and glCopyPixels is how we get to that. I imagine the same 
is

true for other GPUs.

As this operation is critical for many existing X applications for
scrolling data around, we really have to make sure that we hit the
special hardware.


But we can't know if the driver will use this hardware. Maybe with some 
special GL extension?
But also in this way, I'd suggest to use a modified glCopyImageSubData 
and/or glBlitFramebuffer to allow overlapping copys.

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

Re: [PATCH 07/10] glamor: Add glamor_program PolyPoint implementation

[Markus Wick]

Am 2014-03-20 23:35, schrieb Keith Packard:

We are used to also disable the gl program here unnecessaryly.
I want to strip them all, so I'm fine without it, but it isn't fair 
for

a performance comparison.


I stripped them out in an earlier patch; we could measure performance 
at

that point?


I've seen a 40% performance boost in x11perf -dot
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 14/20] glamor: stub out lines

[Markus Wick]

Am 2014-03-20 23:22, schrieb Keith Packard:
I think this functions are worth to reimplement. eg horizontal lines 
are

used a lot and very slow on mi.


Having sped up the underlying functions, they're not terrible at this
point:

200 reps @   0.0035 msec (288000.0/sec): 500-pixel horizontal
line segment

But, yeah, doing a 'real' implementation would be awesome. For
comparison, fbdev does:

   3000 reps @   0.0002 msec (485.0/sec): 500-pixel horizontal
line segment


Sorry, my bad, I wanted to talk about vertical lines:
1000 trep @   0.0046 msec (219000.0/sec): 500-pixel horizontal line 
segment
250 trep @   0.0119 msec ( 84100.0/sec): 500-pixel vertical line 
segment



How are the interpolation requirement for a line? Is it possible to
achive them with an opengl quad?


zero-width line pixelization semantics are pretty weak and we should be
able to hit them with GL quads. Getting the 'notlast' cap semantic
working is about the only trick necessary.


It shouldn't be that hard:

in vec4 pos //x1,y1,x2,y2
in int notlast;
const float line_width = 1.0;

void main() {
  vec2 direction = 0.5 * normalize(vec2(x2,y2) - vec2(x1,y1));
  vec2 normal = vec2(-direction.y, direction.x) * line_width;
  vec2 pos = (gl_VertexId1==0) ? pos.xy - direction : notlast ? pos.zw 
- direction : pos.zw + direction;

  pos += (gl_VertexId2==0) ? -normal : normal;
  gl_Position = vec4(pos, 0.0, 1.0);
}

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

Re: [PATCH 06/20] glamor: Add infrastructure for generating shaders on the fly

[Markus Wick]

Am 2014-03-19 06:09, schrieb Keith Packard:

This just adds a bunch of support code to construct shaders from
'facets', which bundle attributes needed for each layer of the
rendering system. At this point, that includes only the primtive and
the fill stuff.

Signed-off-by: Keith Packard kei...@keithp.com
---
 glamor/Makefile.am|   2 +
 glamor/glamor_priv.h  |   1 +
 glamor/glamor_program.c   | 394 
++

 glamor/glamor_program.h   |  94 +++
 glamor/glamor_transform.c | 216 +
 glamor/glamor_transform.h |  87 ++
 6 files changed, 794 insertions(+)
 create mode 100644 glamor/glamor_program.c
 create mode 100644 glamor/glamor_program.h
 create mode 100644 glamor/glamor_transform.c
 create mode 100644 glamor/glamor_transform.h

diff --git a/glamor/Makefile.am b/glamor/Makefile.am
index ffc8c23..dec6467 100644
--- a/glamor/Makefile.am
+++ b/glamor/Makefile.am
@@ -22,6 +22,8 @@ libglamor_la_SOURCES = \
glamor_setspans.c \
glamor_render.c \
glamor_gradient.c \
+   glamor_program.c \
+   glamor_transform.c \
glamor_trapezoid.c \
glamor_tile.c \
glamor_triangles.c\
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index fc9ab9e..5013799 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -52,6 +52,7 @@

 #include glamor_debug.h
 #include glamor_context.h
+#include glamor_program.h

 #include list.h

diff --git a/glamor/glamor_program.c b/glamor/glamor_program.c
new file mode 100644
index 000..0f9b248
--- /dev/null
+++ b/glamor/glamor_program.c
@@ -0,0 +1,394 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software 
and its
+ * documentation for any purpose is hereby granted without fee, 
provided that
+ * the above copyright notice appear in all copies and that both that 
copyright
+ * notice and this permission notice appear in supporting 
documentation, and
+ * that the name of the copyright holders not be used in advertising 
or
+ * publicity pertaining to distribution of the software without 
specific,
+ * written prior permission.  The copyright holders make no 
representations
+ * about the suitability of this software for any purpose.  It is 
provided as

+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS 
SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN 
NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, 
INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS 
OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR 
OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR 
PERFORMANCE

+ * OF THIS SOFTWARE.
+ */
+
+#include glamor_priv.h
+#include glamor_transform.h
+#include glamor_program.h
+
+static Bool
+use_solid(PixmapPtr pixmap, GCPtr gc, glamor_program *prog, void *arg)
+{
+return glamor_set_solid(pixmap, gc, TRUE, prog-fg_uniform);
+}
+
+const glamor_facet glamor_fill_solid = {
+.name = solid,
+.fs_exec =gl_FragColor = fg;\n,
+.locations = glamor_program_location_fg,
+.use = use_solid,
+};
+
+static Bool
+use_tile(PixmapPtr pixmap, GCPtr gc, glamor_program *prog, void *arg)
+{
+return glamor_set_tiled(pixmap, gc, prog-fill_offset_uniform,
prog-fill_size_uniform);
+}
+
+static const glamor_facet glamor_fill_tile = {
+.name = tile,
+.version = 130,
+.vs_exec = fill_pos = fill_offset + primitive.xy + 
pos;\n,

+.fs_exec = gl_FragColor = texelFetch(sampler,
ivec2(mod(fill_pos,fill_size)), 0);\n,
+.locations = glamor_program_location_fill,
+.use = use_tile,
+};


I think it's better to access gl_FragCoord, so you don't have to add a 
new varying.



+
+#if 0
+static Bool
+use_stipple(PixmapPtr pixmap, GCPtr gc, glamor_program *prog)
+{
+return glamor_set_stippled(pixmap, gc, prog-fg_uniform,
prog-fill_offset_uniform, prog-fill_size_uniform);
+}
+
+static const glamor_facet glamor_fill_stipple = {
+.name = stipple,
+.version = 130,
+.vs_exec = fill_pos = fill_offset + primitive.xy + 
pos;\n;

+.fs_exec = (   if (texelFetch(sampler,
ivec2(mod(fill_pos,fill_size)), 0).x == 0)\n
+   discard;\n
+   gl_FragColor = fg;\n)
+.locations = glamor_program_location_fg | 
glamor_program_location_fill

+.use = use_stipple,
+};
+
+static const glamor_facet glamor_fill_opaque_stipple = {
+.name = opaque_stipple,
+.version = 130,
+.vs_exec = fill_pos = fill_offset + primitive.xy + 
pos;\n;

+.fs_exec = (   if (texelFetch(sampler,
ivec2(mod(fill_pos,fill_size)), 0).x == 0)\n
+   gl_FragColor = bg;\n
+   else\n
+   gl_FragColor = fg;\n),
+

Re: [PATCH 07/20] glamor: Add simple upload/download functions in glamor_transfer

[Markus Wick]

Am 2014-03-19 06:09, schrieb Keith Packard:

These use glTexSubimage2D for upload and glReadPixels for
download. There are a variety of interfaces to the basic function as
needed by the callers.

Signed-off-by: Keith Packard kei...@keithp.com
---
 glamor/Makefile.am   |   2 +
 glamor/glamor_transfer.c | 260 
+++

 glamor/glamor_transfer.h |  55 ++
 3 files changed, 317 insertions(+)
 create mode 100644 glamor/glamor_transfer.c
 create mode 100644 glamor/glamor_transfer.h

diff --git a/glamor/Makefile.am b/glamor/Makefile.am
index dec6467..9544923 100644
--- a/glamor/Makefile.am
+++ b/glamor/Makefile.am
@@ -23,6 +23,8 @@ libglamor_la_SOURCES = \
glamor_render.c \
glamor_gradient.c \
glamor_program.c \
+   glamor_transfer.c \
+   glamor_transfer.h \
glamor_transform.c \
glamor_trapezoid.c \
glamor_tile.c \
diff --git a/glamor/glamor_transfer.c b/glamor/glamor_transfer.c
new file mode 100644
index 000..d25c659
--- /dev/null
+++ b/glamor/glamor_transfer.c
@@ -0,0 +1,260 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software 
and its
+ * documentation for any purpose is hereby granted without fee, 
provided that
+ * the above copyright notice appear in all copies and that both that 
copyright
+ * notice and this permission notice appear in supporting 
documentation, and
+ * that the name of the copyright holders not be used in advertising 
or
+ * publicity pertaining to distribution of the software without 
specific,
+ * written prior permission.  The copyright holders make no 
representations
+ * about the suitability of this software for any purpose.  It is 
provided as

+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS 
SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN 
NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, 
INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS 
OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR 
OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR 
PERFORMANCE

+ * OF THIS SOFTWARE.
+ */
+
+#include glamor_priv.h
+#include glamor_transfer.h
+
+/* XXX a kludge for now */
+void
+glamor_format_for_pixmap(PixmapPtr pixmap, GLenum *format, GLenum 
*type)

+{
+switch (pixmap-drawable.depth) {
+case 24:
+case 32:
+*format = GL_BGRA;
+*type = GL_UNSIGNED_INT_8_8_8_8_REV;
+break;
+case 16:
+*format = GL_RGB;
+*type = GL_UNSIGNED_SHORT_5_6_5;
+break;
+case 15:
+*format = GL_BGRA;
+*type = GL_UNSIGNED_SHORT_1_5_5_5_REV;
+break;
+case 8:
+*format = GL_ALPHA;
+*type = GL_UNSIGNED_BYTE;
+break;
+default:
+FatalError(Invalid pixmap depth %d\n, 
pixmap-drawable.depth);

+break;
+}
+}
+
+/*
+ * Write a region of bits into a pixmap
+ */
+void
+glamor_upload_boxes(PixmapPtr pixmap, BoxPtr in_boxes, int in_nbox,
+int dx_src, int dy_src,
+int dx_dst, int dy_dst,
+uint8_t *bits, uint32_t byte_stride)
+{
+ScreenPtr   screen = pixmap-drawable.pScreen;
+glamor_screen_private   *glamor_priv =
glamor_get_screen_private(screen);
+glamor_pixmap_private   *priv = 
glamor_get_pixmap_private(pixmap);

+int box_x, box_y;
+int bytes_per_pixel =
pixmap-drawable.bitsPerPixel  3;
+GLenum  type;
+GLenum  format;
+
+glamor_format_for_pixmap(pixmap, format, type);
+
+glamor_get_context(glamor_priv);
+
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+
+glPixelStorei(GL_UNPACK_ALIGNMENT, 4);
+glPixelStorei(GL_UNPACK_ROW_LENGTH, byte_stride / 
bytes_per_pixel);

+
+glamor_pixmap_loop(priv, box_x, box_y) {
+BoxPtr  box = glamor_pixmap_box_at(priv, 
box_x, box_y);

+glamor_pixmap_fbo   *fbo = glamor_pixmap_fbo_at(priv,
box_x, box_y);
+BoxPtr  boxes = in_boxes;
+int nbox = in_nbox;
+
+glActiveTexture(GL_TEXTURE0);
+glBindTexture(GL_TEXTURE_2D, fbo-tex);
+
+while (nbox--) {
+
+/* compute drawable coordinates */
+int x1 = boxes-x1 + dx_dst;
+int x2 = boxes-x2 + dx_dst;
+int y1 = boxes-y1 + dy_dst;
+int y2 = boxes-y2 + dy_dst;
+
+if (x1  box-x1)
+x1 = box-x1;
+if (box-x2  x2)
+x2 = box-x2;


x1 = MAX(x1, box-x1);
x2 = MIN(x2, box-x2);


+
+if (x2 = x1)
+

Re: [PATCH 09/20] glamor: Add glamor_program PolyPoint implementation

[Markus Wick]

Am 2014-03-19 06:09, schrieb Keith Packard:

This accelerates poly point when possible by off-loading all geometry
computation to the GPU.

Signed-off-by: Keith Packard kei...@keithp.com
---
 glamor/glamor_polyops.c | 92 
++---

 glamor/glamor_priv.h|  3 ++
 2 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/glamor/glamor_polyops.c b/glamor/glamor_polyops.c
index 1484d80..852e832 100644
--- a/glamor/glamor_polyops.c
+++ b/glamor/glamor_polyops.c
@@ -27,17 +27,101 @@
  */

 #include glamor_priv.h
+#include glamor_transform.h
+
+static const glamor_facet glamor_facet_point = {
+.name = poly_point,
+.vs_vars = attribute vec2 primitive;\n,
+.vs_exec = GLAMOR_POS(gl_Position, primitive),
+};

 static Bool
-_glamor_poly_point(DrawablePtr pDrawable, GCPtr pGC, int mode, int 
npt,

+_glamor_poly_point(DrawablePtr drawable, GCPtr gc, int mode, int npt,
DDXPointPtr ppt, Bool fallback)
 {
-if (!fallback  glamor_ddx_fallback_check_gc(pGC)
- glamor_ddx_fallback_check_pixmap(pDrawable))
+ScreenPtr screen = drawable-pScreen;
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
+glamor_program *prog = glamor_priv-point_prog;
+glamor_pixmap_private *pixmap_priv;
+int off_x, off_y;
+GLshort *vbo_ppt;
+char *vbo_offset;
+int box_x, box_y;
+
+if (!fallback  glamor_ddx_fallback_check_gc(gc)
+ glamor_ddx_fallback_check_pixmap(drawable))
 return FALSE;

-miPolyPoint(pDrawable, pGC, mode, npt, ppt);
+pixmap_priv = glamor_get_pixmap_private(pixmap);
+if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
+goto bail;
+
+glamor_get_context(glamor_priv);
+
+if (prog-failed)
+goto bail_ctx;


I'd move glamor_get_context here. There is no need in calling get + put 
context when the shader isn't available.



+
+if (!prog-prog) {
+if (!glamor_build_program(screen, prog,
+  glamor_facet_point,
+  glamor_fill_solid))
+goto bail_ctx;
+}
+
+if (!glamor_use_program(pixmap, gc, prog, NULL))
+goto bail_ctx;
+
+vbo_ppt = glamor_get_vbo_space(screen, npt * (2 * sizeof
(INT16)), vbo_offset);
+glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
+glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_SHORT, GL_FALSE,
0, vbo_offset);
+if (mode == CoordModePrevious) {
+int n = npt;
+INT16 x = 0, y = 0;
+while (n--) {
+vbo_ppt[0] = (x += ppt-x);
+vbo_ppt[1] = (y += ppt-y);
+vbo_ppt += 2;
+ppt++;
+}
+} else
+memcpy(vbo_ppt, ppt, npt * (2 * sizeof (INT16)));
+glamor_put_vbo_space(screen);
+
+glEnable(GL_SCISSOR_TEST);
+
+glamor_pixmap_loop(pixmap_priv, box_x, box_y) {
+int nbox = RegionNumRects(gc-pCompositeClip);
+BoxPtr box = RegionRects(gc-pCompositeClip);
+
+glamor_set_destination_drawable(drawable, box_x, box_y, TRUE,
TRUE, prog-matrix_uniform, off_x, off_y);
+
+while (nbox--) {
+glScissor(box-x1 + off_x,
+  box-y1 + off_y,
+  box-x2 - box-x1,
+  box-y2 - box-y1);
+box++;
+glDrawArrays(GL_POINTS, 0, npt);
+}
+}
+
+glDisable(GL_SCISSOR_TEST);
+glDisable(GL_COLOR_LOGIC_OP);


Is it allowed to draw the same pixel more than once?
I wonder about overlapping boxes + logic ops.


+glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
+
+glamor_put_context(glamor_priv);
+
+glamor_priv-state = RENDER_STATE;
+glamor_priv-render_idle_cnt = 0;
+
+return TRUE;

+bail_ctx:
+glDisable(GL_COLOR_LOGIC_OP);
+glamor_put_context(glamor_priv);
+bail:
+miPolyPoint(drawable, gc, mode, npt, ppt);
 return TRUE;
 }

diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 1f6df3d..e4be4f3 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -219,6 +219,9 @@ typedef struct glamor_screen_private {
 GLint solid_prog;
 GLint solid_color_uniform_location;

+/* glamor point shader */
+glamor_program point_prog;
+
 /* vertext/elment_index buffer object for render */
 GLuint vbo, ebo;
 /** Next offset within the VBO that glamor_get_vbo_space() will 
use. */

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

Re: [PATCH 10/20] glamor: Add glamor_program based fill/set/get spans

[Markus Wick]

Am 2014-03-19 06:09, schrieb Keith Packard:

This accelerates spans operations using GPU-based geometry computation

Signed-off-by: Keith Packard kei...@keithp.com
---
 glamor/Makefile.am|   1 +
 glamor/glamor.c   |   2 +-
 glamor/glamor_core.c  |   4 +-
 glamor/glamor_priv.h  |  18 +++
 glamor/glamor_spans.c | 299 
++

 5 files changed, 321 insertions(+), 3 deletions(-)
 create mode 100644 glamor/glamor_spans.c

diff --git a/glamor/Makefile.am b/glamor/Makefile.am
index 361c0d6..d598f95 100644
--- a/glamor/Makefile.am
+++ b/glamor/Makefile.am
@@ -25,6 +25,7 @@ libglamor_la_SOURCES = \
glamor_prepare.c \
glamor_prepare.h \
glamor_program.c \
+   glamor_spans.c \
glamor_transfer.c \
glamor_transfer.h \
glamor_transform.c \
diff --git a/glamor/glamor.c b/glamor/glamor.c
index 79914bf..abf2d47 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -426,7 +426,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 screen-DestroyPixmap = glamor_destroy_pixmap;

 glamor_priv-saved_procs.get_spans = screen-GetSpans;
-screen-GetSpans = glamor_get_spans;
+screen-GetSpans = glamor_getspans;

 glamor_priv-saved_procs.get_image = screen-GetImage;
 screen-GetImage = glamor_get_image;
diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
index 6c0b3c8..7983bd9 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -413,8 +413,8 @@ glamor_stipple(PixmapPtr pixmap, PixmapPtr stipple,
 }

 GCOps glamor_gc_ops = {
-.FillSpans = glamor_fill_spans,
-.SetSpans = glamor_set_spans,
+.FillSpans = glamor_fillspans,
+.SetSpans = glamor_setspans,
 .PutImage = glamor_put_image,
 .CopyArea = glamor_copy_area,
 .CopyPlane = glamor_copy_plane,
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index e4be4f3..b3b1ac7 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -222,6 +222,9 @@ typedef struct glamor_screen_private {
 /* glamor point shader */
 glamor_program point_prog;

+/* glamor spans shaders */
+glamor_program_fill fill_spans_program;
+
 /* vertext/elment_index buffer object for render */
 GLuint vbo, ebo;
 /** Next offset within the VBO that glamor_get_vbo_space() will 
use. */

@@ -974,6 +977,21 @@ RegionPtr glamor_copy_plane(DrawablePtr pSrc,
DrawablePtr pDst, GCPtr pGC,
 int dstx, int dsty,
 unsigned long bitPlane);

+/* glamor_spans.c */
+void
+glamor_fillspans(DrawablePtr drawable,
+ GCPtr gc,
+ int n, DDXPointPtr points, int *widths, int sorted);
+
+void
+glamor_getspans(DrawablePtr drawable, int wmax,
+DDXPointPtr points, int *widths, int count, char 
*dst);

+
+void
+glamor_setspans(DrawablePtr drawable, GCPtr gc, char *src,
+DDXPointPtr points, int *widths, int numPoints, int 
sorted);

+
+/* glamor_glyphblt.c */
 void glamor_image_glyph_blt(DrawablePtr pDrawable, GCPtr pGC,
 int x, int y, unsigned int nglyph,
 CharInfoPtr *ppci, void *pglyphBase);
diff --git a/glamor/glamor_spans.c b/glamor/glamor_spans.c
new file mode 100644
index 000..6cb635b
--- /dev/null
+++ b/glamor/glamor_spans.c
@@ -0,0 +1,299 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software 
and its
+ * documentation for any purpose is hereby granted without fee, 
provided that
+ * the above copyright notice appear in all copies and that both that 
copyright
+ * notice and this permission notice appear in supporting 
documentation, and
+ * that the name of the copyright holders not be used in advertising 
or
+ * publicity pertaining to distribution of the software without 
specific,
+ * written prior permission.  The copyright holders make no 
representations
+ * about the suitability of this software for any purpose.  It is 
provided as

+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS 
SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN 
NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, 
INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS 
OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR 
OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR 
PERFORMANCE

+ * OF THIS SOFTWARE.
+ */
+
+#include glamor_priv.h
+#include glamor_transform.h
+#include glamor_transfer.h
+#include glamor_prepare.h
+
+glamor_program  fill_spans_progs[4];
+
+static const glamor_facet glamor_facet_fillspans = {
+.name = fill_spans,
+.version = 130,
+.vs_vars =  attribute vec3 primitive;\n,
+.vs_exec = (   vec2 pos = vec2(primitive.z,1) *
vec2(gl_VertexID1, (gl_VertexID2)1);\n
+  

Re: [PATCH 13/20] glamor: Add glamor_program based copy acceleration

[Markus Wick]

Am 2014-03-19 06:09, schrieb Keith Packard:

Uses glCopyPixels for self-copies, otherwise paints with textures.
Performs CPU to GPU transfers for pixmaps in memory.
Accelerates copy plane when both objects are in memory
Includes copy_window acceleration too.

Signed-off-by: Keith Packard kei...@keithp.com
---
 glamor/Makefile.am  |   1 +
 glamor/glamor.c |   2 +-
 glamor/glamor_copy.c| 510 


 glamor/glamor_core.c|   4 +-
 glamor/glamor_priv.h|  30 +++
 glamor/glamor_program.c |  10 +-
 glamor/glamor_program.h |   3 +
 7 files changed, 556 insertions(+), 4 deletions(-)
 create mode 100644 glamor/glamor_copy.c

diff --git a/glamor/Makefile.am b/glamor/Makefile.am
index f859155..7f756dc 100644
--- a/glamor/Makefile.am
+++ b/glamor/Makefile.am
@@ -7,6 +7,7 @@ AM_CFLAGS = $(CWARNFLAGS) $(DIX_CFLAGS) 
$(GLAMOR_CFLAGS)

 libglamor_la_SOURCES = \
glamor.c \
glamor_context.h \
+   glamor_copy.c \
glamor_copyarea.c \
glamor_copywindow.c \
glamor_core.c \
diff --git a/glamor/glamor.c b/glamor/glamor.c
index dd18386..3117013 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -439,7 +439,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 screen-ChangeWindowAttributes = 
glamor_change_window_attributes;


 glamor_priv-saved_procs.copy_window = screen-CopyWindow;
-screen-CopyWindow = glamor_copy_window;
+screen-CopyWindow = glamor_copywindow;

 glamor_priv-saved_procs.bitmap_to_region = 
screen-BitmapToRegion;

 screen-BitmapToRegion = glamor_bitmap_to_region;
diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c
new file mode 100644
index 000..a26c4a7
--- /dev/null
+++ b/glamor/glamor_copy.c
@@ -0,0 +1,510 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software 
and its
+ * documentation for any purpose is hereby granted without fee, 
provided that
+ * the above copyright notice appear in all copies and that both that 
copyright
+ * notice and this permission notice appear in supporting 
documentation, and
+ * that the name of the copyright holders not be used in advertising 
or
+ * publicity pertaining to distribution of the software without 
specific,
+ * written prior permission.  The copyright holders make no 
representations
+ * about the suitability of this software for any purpose.  It is 
provided as

+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS 
SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN 
NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, 
INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS 
OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR 
OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR 
PERFORMANCE

+ * OF THIS SOFTWARE.
+ */
+
+#include glamor_priv.h
+#include glamor_transfer.h
+#include glamor_prepare.h
+#include glamor_transform.h
+
+struct copy_args {
+PixmapPtr   src_pixmap;
+glamor_pixmap_fbo   *src;
+uint32_tbitplane;
+int dx, dy;
+};
+
+static Bool
+use_copyarea(PixmapPtr dst, GCPtr gc, glamor_program *prog, void *arg)
+{
+struct copy_args *args = arg;
+glamor_pixmap_fbo *src = args-src;
+
+glActiveTexture(GL_TEXTURE0);
+glBindTexture(GL_TEXTURE_2D, src-tex);
+
+glUniform2f(prog-fill_offset_uniform, args-dx, args-dy);
+glUniform2f(prog-fill_size_uniform, src-width, src-height);
+
+return TRUE;
+}
+
+static const glamor_facet glamor_facet_copyarea = {
+copy_area,
+.version = 130,
+.vs_vars = attribute vec4 primitive;\n,
+.vs_exec = (   vec2 pos = (primitive.zw - primitive.xy) *
vec2(gl_VertexID1, (gl_VertexID2)1);\n
+GLAMOR_POS(gl_Position, (primitive.xy + pos))
+   fill_pos = fill_offset + primitive.xy + 
pos;\n),

+.fs_exec =gl_FragColor = texelFetch(sampler,
ivec2(fill_pos), 0);\n,
+.locations = glamor_program_location_fill,
+.use = use_copyarea,
+};
+
+static Bool
+use_copyplane(PixmapPtr dst, GCPtr gc, glamor_program *prog, void 
*arg)

+{
+struct copy_args *args = arg;
+glamor_pixmap_fbo *src = args-src;
+
+glActiveTexture(GL_TEXTURE0);
+glBindTexture(GL_TEXTURE_2D, src-tex);
+
+glUniform2f(prog-fill_offset_uniform, args-dx, args-dy);
+glUniform2f(prog-fill_size_uniform, src-width, src-height);
+
+glamor_set_color(dst, gc-fgPixel, prog-fg_uniform);
+glamor_set_color(dst, gc-bgPixel, prog-bg_uniform);
+
+/* XXX handle 2 10 10 10 and 1555 formats; presumably the pixmap
private knows this? */
+switch (args-src_pixmap-drawable.depth) {
+case 24:
+glUniform4ui(prog-bitplane_uniform,
+ (args-bitplane  16)  0xff,
+   

Re: [PATCH 14/20] glamor: stub out lines

[Markus Wick]

I think this functions are worth to reimplement. eg horizontal lines are 
used a lot and very slow on mi.
How are the interpolation requirement for a line? Is it possible to 
achive them with an opengl quad?


Am 2014-03-19 06:09, schrieb Keith Packard:

Use mi line code for now

Signed-off-by: Keith Packard kei...@keithp.com
---
 glamor/glamor_core.c | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
index f350746..1536c54 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -412,6 +412,25 @@ glamor_stipple(PixmapPtr pixmap, PixmapPtr 
stipple,

 return FALSE;
 }

+static void
+glamor_fallback_poly_line(DrawablePtr pDrawable, GCPtr pGC, int mode,
int npt, DDXPointPtr ppt)
+{
+void (*line) (DrawablePtr, GCPtr, int mode, int npt, DDXPointPtr 
ppt);

+
+if (pGC-lineWidth == 0) {
+if (pGC-lineStyle != LineSolid)
+line = miZeroDashLine;
+else
+line = miZeroLine;
+} else {
+if (pGC-lineStyle != LineSolid)
+line = miWideDash;
+else
+line = miWideLine;
+}
+(*line) (pDrawable, pGC, mode, npt, ppt);
+}
+
 GCOps glamor_gc_ops = {
 .FillSpans = glamor_fillspans,
 .SetSpans = glamor_setspans,
@@ -419,8 +438,8 @@ GCOps glamor_gc_ops = {
 .CopyArea = glamor_copyarea,
 .CopyPlane = glamor_copyplane,
 .PolyPoint = glamor_poly_point,
-.Polylines = glamor_poly_lines,
-.PolySegment = glamor_poly_segment,
+.Polylines = glamor_fallback_poly_line,
+.PolySegment = miPolySegment,
 .PolyRectangle = miPolyRectangle,
 .PolyArc = miPolyArc,
 .FillPolygon = miFillPolygon,

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

Re: [PATCH 00/20] New glamor core rendering code (v2)

[Markus Wick]

This patch series is a very nice cleanup. I like the new way how this 
functions should be implemented, but the shader generator. Plain GLSL + 
predefined header are less confusing and more flexible imo.


We also have to figure out on which gl extension we want to rely on. eg 
I don't think we should fall back to fb because glsl130 + 
ARB_instanced_arrays aren't available.


Somehow I feel like you haven't seen my review mails on your first 
attempt of this series, have you?


Am 2014-03-19 06:09, schrieb Keith Packard:

This replaces all of the X core rendering code in glamor with shiny
new stuff. This is the second time around for this code, it now passes
the X test suite for Xlib chapter 9 (thanks to Eric's work making that
run under piglit). And, it also now runs correctly when debugging with
tiny FBOs tiling the pixmaps.

There are  two core X server bug fixes at the head of this list,
followed by some simple fixes to the core of glamor. Then a pile of
new rendering code, followed by the removal of some of the
old.

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

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

[PATCH 02/10] glamor: Use glsl fract/mod instead of while in gradient shaders.

[Markus Wick]

This fixes gtkperf. It seemed to hang forever.
---
 glamor/glamor_gradient.c | 48 
 1 file changed, 4 insertions(+), 44 deletions(-)

diff --git a/glamor/glamor_gradient.c b/glamor/glamor_gradient.c
index f77d6a8..38dac68 100644
--- a/glamor/glamor_gradient.c
+++ b/glamor/glamor_gradient.c
@@ -247,7 +247,6 @@ _glamor_create_radial_gradient_program(ScreenPtr screen, 
int stops_count,
{\n\
float t = 0.0;\n\
float sqrt_value;\n\
-   int revserse = 0;\n\
t_invalid = 0;\n\
\n\
vec3 tmp = vec3(source_texture.x, source_texture.y, 1.0);\n\
@@ -295,30 +294,11 @@ _glamor_create_radial_gradient_program(ScreenPtr screen, 
int stops_count,
}\n\
\n\
if(repeat_type == %d){\n /* repeat normal*/\
-   while(t  1.0) \n\
-   t = t - 1.0; \n\
-   while(t  0.0) \n\
-   t = t + 1.0; \n\
+   t = fract(t);\n\
}\n\
\n\
if(repeat_type == %d) {\n /* repeat reflect*/\
-   while(t  1.0) {\n\
-   t = t - 1.0; \n\
-   if(revserse == 0)\n\
-   revserse = 1;\n\
-   else\n\
-   revserse = 0;\n\
-   }\n\
-   while(t  0.0) {\n\
-   t = t + 1.0; \n\
-   if(revserse == 0)\n\
-   revserse = 1;\n\
-   else\n\
-   revserse = 0;\n\
-   }\n\
-   if(revserse == 1) {\n\
-   t = 1.0 - t; \n\
-   }\n\
+   t = abs(fract(t * 0.5 + 0.5) * 2.0 - 1.0);\n\
}\n\
\n\
return t;\n\
@@ -492,7 +472,6 @@ _glamor_create_linear_gradient_program(ScreenPtr screen, 
int stops_count,
vec4 stop_color_before;\n\
vec4 stop_color_after;\n\
float new_alpha; \n\
-   int revserse = 0;\n\
vec4 gradient_color;\n\
float percentage; \n\
vec3 source_texture_trans = transform_mat * tmp;\n\
@@ -512,30 +491,11 @@ _glamor_create_linear_gradient_program(ScreenPtr screen, 
int stops_count,
distance = distance - _p1_distance; \n\
\n\
if(repeat_type == %d){\n /* repeat normal*/\
-   while(distance  _pt_distance) \n\
-   distance = distance - (_pt_distance); \n\
-   while(distance  0.0) \n\
-   distance = distance + (_pt_distance); \n\
+   distance = mod(distance, _pt_distance);\n\
}\n\
\n\
if(repeat_type == %d) {\n /* repeat reflect*/\
-   while(distance  _pt_distance) {\n\
-   distance = distance - (_pt_distance); \n\
-   if(revserse == 0)\n\
-   revserse = 1;\n\
-   else\n\
-   revserse = 0;\n\
-   }\n\
-   while(distance  0.0) {\n\
-   distance = distance + (_pt_distance); \n\
-   if(revserse == 0)\n\
-   revserse = 1;\n\
-   else\n\
-   revserse = 0;\n\
-   }\n\
-   if(revserse == 1) {\n\
-   distance = (_pt_distance) - distance; \n\
-   }\n\
+   distance = abs(mod(distance + _pt_distance, 2.0 * 
_pt_distance) - _pt_distance);\n\
}\n\
\n\
len_percentage = distance/(_pt_distance);\n\
-- 
1.9.0

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

[PATCH 05/10] glamor: Update GL requirements to 2.1.

[Markus Wick]

We will never ever run on OpenGL 1.2 as we use shaders everywhere.
2.0 may be enough, but we also often use PBOs and our big shaders
won't fit into the first GLSL limits.
---
 glamor/glamor.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index ef969e2..eb13430 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -357,8 +357,8 @@ glamor_init(ScreenPtr screen, unsigned int flags)
  * Windows with Intel 4-series (G45) graphics or older.
  */
 if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
-if (gl_version  13) {
-ErrorF(Require OpenGL version 1.3 or later.\n);
+if (gl_version  21) {
+ErrorF(Require OpenGL version 2.1 or later.\n);
 goto fail;
 }
 } else {
-- 
1.9.0

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

[PATCH 10/10] glamor: Use vbo for solid boxes.

[Markus Wick]

---
 glamor/glamor_fill.c | 40 
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/glamor/glamor_fill.c b/glamor/glamor_fill.c
index 2fa726e..3286aa1 100644
--- a/glamor/glamor_fill.c
+++ b/glamor/glamor_fill.c
@@ -190,9 +190,6 @@ _glamor_solid_boxes(PixmapPtr pixmap, BoxPtr box, int nbox, 
float *color)
 glamor_screen_private *glamor_priv = glamor_get_screen_private(screen);
 glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap);
 GLfloat xscale, yscale;
-float stack_vertices[32];
-float *vertices = stack_vertices;
-int valid_nbox = ARRAY_SIZE(stack_vertices) / (4 * 2);
 
 glamor_set_destination_pixmap_priv_nc(pixmap_priv);
 
@@ -203,39 +200,29 @@ _glamor_solid_boxes(PixmapPtr pixmap, BoxPtr box, int 
nbox, float *color)
 
 pixmap_priv_get_dest_scale(pixmap_priv, xscale, yscale);
 
-if (nbox  valid_nbox) {
-int allocated_nbox;
-float *new_vertices;
-
-if (nbox  GLAMOR_COMPOSITE_VBO_VERT_CNT / 6)
-allocated_nbox = GLAMOR_COMPOSITE_VBO_VERT_CNT / 6;
-else
-allocated_nbox = nbox;
-new_vertices = malloc(allocated_nbox * 4 * 2 * sizeof(float));
-if (new_vertices) {
-vertices = new_vertices;
-valid_nbox = allocated_nbox;
-}
-}
-
-glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_FLOAT,
-  GL_FALSE, 2 * sizeof(float), vertices);
 glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
 
 while (nbox) {
 int box_cnt, i;
-float *next_box;
+float *vertices;
+char *vbo_offset;
 
-next_box = vertices;
-box_cnt = nbox  valid_nbox ? valid_nbox : nbox;
+box_cnt = MIN(nbox, GLAMOR_COMPOSITE_VBO_VERT_CNT / 6);
+
+vertices = glamor_get_vbo_space(screen,
+box_cnt * 4 * 2 * sizeof(float),
+vbo_offset);
+glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_FLOAT,
+   GL_FALSE, 2 * sizeof(float), vbo_offset);
 for (i = 0; i  box_cnt; i++) {
 glamor_set_normalize_vcoords(pixmap_priv, xscale, yscale,
  box[i].x1, box[i].y1,
  box[i].x2, box[i].y2,
  glamor_priv-yInverted,
- next_box);
-next_box += 4 * 2;
+ vertices);
+vertices += 4 * 2;
 }
+glamor_put_vbo_space(screen);
 if (box_cnt == 1)
 glDrawArrays(GL_TRIANGLE_FAN, 0, box_cnt * 4);
 else {
@@ -251,9 +238,6 @@ _glamor_solid_boxes(PixmapPtr pixmap, BoxPtr box, int nbox, 
float *color)
 box += box_cnt;
 }
 
-if (vertices != stack_vertices)
-free(vertices);
-
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glamor_put_context(glamor_priv);
 glamor_priv-state = RENDER_STATE;
-- 
1.9.0

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

[PATCH 06/10] glamor: Select VBO path by ARB_mbr extension.

[Markus Wick]

The mbr path was hard coded enabled for desktop gl and disabled for gles.
But there are both, desktop without mbr and mobiles with mbr.
---
 glamor/glamor.c  | 2 ++
 glamor/glamor_priv.h | 1 +
 glamor/glamor_vbo.c  | 4 ++--
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index eb13430..0d0f52c 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -378,6 +378,8 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 epoxy_has_gl_extension(GL_MESA_pack_invert);
 glamor_priv-has_fbo_blit =
 epoxy_has_gl_extension(GL_EXT_framebuffer_blit);
+glamor_priv-has_map_buffer_range =
+epoxy_has_gl_extension(GL_ARB_map_buffer_range);
 glamor_priv-has_buffer_storage =
 epoxy_has_gl_extension(GL_ARB_buffer_storage);
 glGetIntegerv(GL_MAX_RENDERBUFFER_SIZE, glamor_priv-max_fbo_size);
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 0f0b0f3..d4d2e75 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -213,6 +213,7 @@ typedef struct glamor_screen_private {
 enum glamor_gl_flavor gl_flavor;
 int has_pack_invert;
 int has_fbo_blit;
+int has_map_buffer_range;
 int has_buffer_storage;
 int has_khr_debug;
 int max_fbo_size;
diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
index 5e98bfe..c6ffea8 100644
--- a/glamor/glamor_vbo.c
+++ b/glamor/glamor_vbo.c
@@ -96,7 +96,7 @@ glamor_get_vbo_space(ScreenPtr screen, unsigned size, char 
**vbo_offset)
 *vbo_offset = (void *)(uintptr_t)glamor_priv-vbo_offset;
 data = glamor_priv-vb + glamor_priv-vbo_offset;
 glamor_priv-vbo_offset += size;
-} else if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
+} else if (glamor_priv-has_map_buffer_range) {
 if (glamor_priv-vbo_size  glamor_priv-vbo_offset + size) {
 glamor_priv-vbo_size = MAX(GLAMOR_VBO_SIZE, size);
 glamor_priv-vbo_offset = 0;
@@ -147,7 +147,7 @@ glamor_put_vbo_space(ScreenPtr screen)
  * persistent mapping, so we can leave it around until we
  * reach the end of the buffer.
  */
-} else if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
+} else if (glamor_priv-has_map_buffer_range) {
 glUnmapBuffer(GL_ARRAY_BUFFER);
 } else {
 glBufferData(GL_ARRAY_BUFFER, glamor_priv-vbo_offset,
-- 
1.9.0

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

[PATCH 03/10] glamor: Use epoxy_has_gl_extension() instead of rolling our own.

[Markus Wick]

---
 glamor/glamor.c  | 10 +-
 glamor/glamor_core.c | 22 --
 glamor/glamor_priv.h |  1 -
 3 files changed, 5 insertions(+), 28 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 0f7d68b..9d171b7 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -367,19 +367,19 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 goto fail;
 }
 
-if (!glamor_gl_has_extension(GL_EXT_texture_format_BGRA)) {
+if (!epoxy_has_gl_extension(GL_EXT_texture_format_BGRA)) {
 ErrorF(GL_EXT_texture_format_BGRA required\n);
 goto fail;
 }
 }
 
-glamor_priv-has_khr_debug = glamor_gl_has_extension(GL_KHR_debug);
+glamor_priv-has_khr_debug = epoxy_has_gl_extension(GL_KHR_debug);
 glamor_priv-has_pack_invert =
-glamor_gl_has_extension(GL_MESA_pack_invert);
+epoxy_has_gl_extension(GL_MESA_pack_invert);
 glamor_priv-has_fbo_blit =
-glamor_gl_has_extension(GL_EXT_framebuffer_blit);
+epoxy_has_gl_extension(GL_EXT_framebuffer_blit);
 glamor_priv-has_buffer_storage =
-glamor_gl_has_extension(GL_ARB_buffer_storage);
+epoxy_has_gl_extension(GL_ARB_buffer_storage);
 glGetIntegerv(GL_MAX_RENDERBUFFER_SIZE, glamor_priv-max_fbo_size);
 #ifdef MAX_FBO_SIZE
 glamor_priv-max_fbo_size = MAX_FBO_SIZE;
diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
index 6c0b3c8..eeaa595 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -559,28 +559,6 @@ glamor_bitmap_to_region(PixmapPtr pixmap)
 return ret;
 }
 
-/* Borrow from cairo. */
-Bool
-glamor_gl_has_extension(const char *extension)
-{
-const char *pext;
-int ext_len;
-
-ext_len = strlen(extension);
-
-pext = (const char *) glGetString(GL_EXTENSIONS);
-
-if (pext == NULL || extension == NULL)
-return FALSE;
-
-while ((pext = strstr(pext, extension)) != NULL) {
-if (pext[ext_len] == ' ' || pext[ext_len] == '\0')
-return TRUE;
-pext += ext_len;
-}
-return FALSE;
-}
-
 int
 glamor_gl_get_version(void)
 {
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index bc7d3f8..f278bef 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -619,7 +619,6 @@ Bool glamor_set_alu(ScreenPtr screen, unsigned char alu);
 Bool glamor_set_planemask(PixmapPtr pixmap, unsigned long planemask);
 Bool glamor_change_window_attributes(WindowPtr pWin, unsigned long mask);
 RegionPtr glamor_bitmap_to_region(PixmapPtr pixmap);
-Bool glamor_gl_has_extension(const char *extension);
 int glamor_gl_get_version(void);
 
 #define GLAMOR_GL_VERSION_ENCODE(major, minor) ( \
-- 
1.9.0

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

[PATCH 07/10] glamor: Don't reset GL_ELEMENT_ARRAY_BUFFER as it's always bound to the same buffer.

[Markus Wick]

---
 glamor/glamor_fill.c  | 3 ---
 glamor/glamor_gradient.c  | 4 
 glamor/glamor_render.c| 6 +-
 glamor/glamor_trapezoid.c | 6 --
 4 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/glamor/glamor_fill.c b/glamor/glamor_fill.c
index 7461b62..2fa726e 100644
--- a/glamor/glamor_fill.c
+++ b/glamor/glamor_fill.c
@@ -218,9 +218,6 @@ _glamor_solid_boxes(PixmapPtr pixmap, BoxPtr box, int nbox, 
float *color)
 }
 }
 
-if (_X_UNLIKELY(nbox  1))
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, glamor_priv-ebo);
-
 glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_FLOAT,
   GL_FALSE, 2 * sizeof(float), vertices);
 glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
diff --git a/glamor/glamor_gradient.c b/glamor/glamor_gradient.c
index 38dac68..ce24d73 100644
--- a/glamor/glamor_gradient.c
+++ b/glamor/glamor_gradient.c
@@ -1121,7 +1121,6 @@ glamor_generate_radial_gradient_picture(ScreenPtr screen,
 }
 
 glBindBuffer(GL_ARRAY_BUFFER, 0);
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
@@ -1142,7 +1141,6 @@ glamor_generate_radial_gradient_picture(ScreenPtr screen,
 }
 
 glBindBuffer(GL_ARRAY_BUFFER, 0);
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
@@ -1472,7 +1470,6 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
 }
 
 glBindBuffer(GL_ARRAY_BUFFER, 0);
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
@@ -1493,7 +1490,6 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
 }
 
 glBindBuffer(GL_ARRAY_BUFFER, 0);
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index c0ee22c..62236b9 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -427,11 +427,9 @@ glamor_init_composite_shaders(ScreenPtr screen)
 
 if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
 glUnmapBuffer(GL_ELEMENT_ARRAY_BUFFER);
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 }
 else {
 glBufferData(GL_ELEMENT_ARRAY_BUFFER, eb_size, eb, GL_STATIC_DRAW);
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 
 free(eb);
 }
@@ -448,6 +446,7 @@ glamor_fini_composite_shaders(ScreenPtr screen)
 
 glamor_priv = glamor_get_screen_private(screen);
 glamor_get_context(glamor_priv);
+glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 glDeleteBuffers(1, glamor_priv-ebo);
 
 for (i = 0; i  SHADER_SOURCE_COUNT; i++)
@@ -706,8 +705,6 @@ glamor_setup_composite_vbo(ScreenPtr screen, int n_verts)
 glamor_get_context(glamor_priv);
 vb = glamor_get_vbo_space(screen, vert_size, vbo_offset);
 
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, glamor_priv-ebo);
-
 glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_FLOAT, GL_FALSE,
   glamor_priv-vb_stride, vbo_offset);
 glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
@@ -1329,7 +1326,6 @@ glamor_composite_with_shader(CARD8 op,
 }
 }
 
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
 glDisableVertexAttribArray(GLAMOR_VERTEX_MASK);
diff --git a/glamor/glamor_trapezoid.c b/glamor/glamor_trapezoid.c
index 969ab68..c76b8bb 100644
--- a/glamor/glamor_trapezoid.c
+++ b/glamor/glamor_trapezoid.c
@@ -637,8 +637,6 @@ glamor_setup_composite_vbo_for_trapezoid(ScreenPtr screen, 
int n_verts)
 
 vb = glamor_get_vbo_space(screen, vert_size, vbo_offset);
 
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, glamor_priv-ebo);
-
 /* Set the vertex pointer. */
 glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_FLOAT,
   GL_FALSE, glamor_priv-vb_stride,
@@ -977,7 +975,6 @@ _glamor_trapezoids_with_shader(CARD8 op,
 ret = TRUE;
 
  TRAPEZOID_RESET_GL:
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
 glDisableVertexAttribArray(GLAMOR_VERTEX_MASK);
@@ -1415,8 +1412,6 @@ _glamor_generate_trapezoid_with_shader(ScreenPtr screen, 
PicturePtr picture,
 
 pixmap_priv_get_dest_scale(pixmap_priv, (xscale), (yscale));
 
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
-
 /* Now draw the Trapezoid mask. */
 glUseProgram(trapezoid_prog);
 
@@ -1562,7 +1557,6 @@ _glamor_generate_trapezoid_with_shader(ScreenPtr screen, 
PicturePtr picture,
 }
 }
 
-glBindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0);
 glBlendFunc(GL_ONE, GL_ZERO);
 glDisable(GL_BLEND);
 

[PATCH 04/10] glamor: Use epoxy_gl_version() instead of rolling our own.

[Markus Wick]

---
 glamor/glamor.c  |  6 +++---
 glamor/glamor_core.c | 23 ---
 glamor/glamor_priv.h |  5 -
 3 files changed, 3 insertions(+), 31 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 9d171b7..ef969e2 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -341,7 +341,7 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 else
 glamor_priv-gl_flavor = GLAMOR_GL_ES2;
 
-gl_version = glamor_gl_get_version();
+gl_version = epoxy_gl_version();
 
 /* We'd like to require GL_ARB_map_buffer_range or
  * GL_OES_map_buffer_range, since it offers more information to
@@ -357,12 +357,12 @@ glamor_init(ScreenPtr screen, unsigned int flags)
  * Windows with Intel 4-series (G45) graphics or older.
  */
 if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
-if (gl_version  GLAMOR_GL_VERSION_ENCODE(1, 3)) {
+if (gl_version  13) {
 ErrorF(Require OpenGL version 1.3 or later.\n);
 goto fail;
 }
 } else {
-if (gl_version  GLAMOR_GL_VERSION_ENCODE(2, 0)) {
+if (gl_version  20) {
 ErrorF(Require Open GLES2.0 or later.\n);
 goto fail;
 }
diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
index eeaa595..5711be7 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -559,26 +559,3 @@ glamor_bitmap_to_region(PixmapPtr pixmap)
 return ret;
 }
 
-int
-glamor_gl_get_version(void)
-{
-int major, minor;
-const char *version = (const char *) glGetString(GL_VERSION);
-const char *dot = version == NULL ? NULL : strchr(version, '.');
-const char *major_start = dot;
-
-/* Sanity check */
-if (dot == NULL || dot == version || *(dot + 1) == '\0') {
-major = 0;
-minor = 0;
-}
-else {
-/* Find the start of the major version in the string */
-while (major_start  version  *major_start != ' ')
---major_start;
-major = strtol(major_start, NULL, 10);
-minor = strtol(dot + 1, NULL, 10);
-}
-
-return GLAMOR_GL_VERSION_ENCODE(major, minor);
-}
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index f278bef..0f0b0f3 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -619,11 +619,6 @@ Bool glamor_set_alu(ScreenPtr screen, unsigned char alu);
 Bool glamor_set_planemask(PixmapPtr pixmap, unsigned long planemask);
 Bool glamor_change_window_attributes(WindowPtr pWin, unsigned long mask);
 RegionPtr glamor_bitmap_to_region(PixmapPtr pixmap);
-int glamor_gl_get_version(void);
-
-#define GLAMOR_GL_VERSION_ENCODE(major, minor) ( \
-  ((major) * 256)   \
-+ ((minor) *   1))
 
 /* glamor_fill.c */
 Bool glamor_fill(DrawablePtr drawable,
-- 
1.9.0

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

[PATCH 08/10] glamor: Remove unneeded unbindings.

[Markus Wick]

They are already cleared in glamor_put_vbo_space.
---
 glamor/glamor_gradient.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/glamor/glamor_gradient.c b/glamor/glamor_gradient.c
index ce24d73..c24f342 100644
--- a/glamor/glamor_gradient.c
+++ b/glamor/glamor_gradient.c
@@ -1120,8 +1120,6 @@ glamor_generate_radial_gradient_picture(ScreenPtr screen,
 free(stop_colors);
 }
 
-glBindBuffer(GL_ARRAY_BUFFER, 0);
-
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
 
@@ -1140,8 +1138,6 @@ glamor_generate_radial_gradient_picture(ScreenPtr screen,
 free(stop_colors);
 }
 
-glBindBuffer(GL_ARRAY_BUFFER, 0);
-
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
 glamor_put_context(glamor_priv);
@@ -1469,8 +1465,6 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
 free(stop_colors);
 }
 
-glBindBuffer(GL_ARRAY_BUFFER, 0);
-
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
 
@@ -1489,8 +1483,6 @@ glamor_generate_linear_gradient_picture(ScreenPtr screen,
 free(stop_colors);
 }
 
-glBindBuffer(GL_ARRAY_BUFFER, 0);
-
 glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
 glDisableVertexAttribArray(GLAMOR_VERTEX_SOURCE);
 glamor_put_context(glamor_priv);
-- 
1.9.0

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

[PATCH 09/10] glamor: Drop feature dependent optimization on startup.

[Markus Wick]

We don't care that much about startup time to write different code paths...
---
 glamor/glamor_render.c | 23 +++
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index 62236b9..cdf8eff 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -412,27 +412,10 @@ glamor_init_composite_shaders(ScreenPtr screen)
 
 eb_size = GLAMOR_COMPOSITE_VBO_VERT_CNT * sizeof(short) * 2;
 
-if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
-glBufferData(GL_ELEMENT_ARRAY_BUFFER, eb_size, NULL, GL_STATIC_DRAW);
-eb = glMapBuffer(GL_ELEMENT_ARRAY_BUFFER, GL_WRITE_ONLY);
-}
-else {
-eb = malloc(eb_size);
-}
-
-if (eb == NULL)
-FatalError
-(fatal error, fail to get element buffer. GL context may be not 
created correctly.\n);
+eb = XNFalloc(eb_size);
 glamor_init_eb(eb, GLAMOR_COMPOSITE_VBO_VERT_CNT);
-
-if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
-glUnmapBuffer(GL_ELEMENT_ARRAY_BUFFER);
-}
-else {
-glBufferData(GL_ELEMENT_ARRAY_BUFFER, eb_size, eb, GL_STATIC_DRAW);
-
-free(eb);
-}
+glBufferData(GL_ELEMENT_ARRAY_BUFFER, eb_size, eb, GL_STATIC_DRAW);
+free(eb);
 
 glamor_put_context(glamor_priv);
 }
-- 
1.9.0

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

[PATCH 00/10] glamor cleanup series

[Markus Wick]

But I still don't know if the last patch is worth as this code will likely be
removed soon by Keith.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 02/10] kdrive: Xv code expects attribute names to be allocated each generation

[Markus Wick]

Yeah, this fixes a crash in
kdrive: Remove duplicated definitions of some XV-related structs..
But as this patch was skipped in the megaseries, there is no need in 
only applying this fix.


Maybe squashing both patches together as the origin was a nice cleanup?

Am 2014-03-14 07:29, schrieb Keith Packard:

Xv frees the attribute names for each X server generation; reset the
server twice without allocating them  again and you get malloc
complaints (or worse).

Signed-off-by: Keith Packard kei...@keithp.com
---
 hw/kdrive/src/kxv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/kdrive/src/kxv.c b/hw/kdrive/src/kxv.c
index 6f3d85e..f5b1e8a 100644
--- a/hw/kdrive/src/kxv.c
+++ b/hw/kdrive/src/kxv.c
@@ -371,6 +371,7 @@ KdXVInitAdaptors(ScreenPtr pScreen,
KdVideoAdaptorPtr * infoPtr, int number)
 for (pat = pAttribute, attributePtr = 
adaptorPtr-pAttributes, i =
  0; i  adaptorPtr-nAttributes; pat++, i++, 
attributePtr++) {

 memcpy(pat, attributePtr, sizeof(*pat));
+pat-name = strdup(pAttribute-name);
 }
 pa-nAttributes = adaptorPtr-nAttributes;
 pa-pAttributes = pAttribute;

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

Re: [PATCH 03/10] glamor: Unbind GL_ARRAY_BUFFER in glamor_put_vbo_space

[Markus Wick]

Am 2014-03-14 07:29, schrieb Keith Packard:

diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
index 5e98bfe..31e0730 100644
--- a/glamor/glamor_vbo.c
+++ b/glamor/glamor_vbo.c
@@ -153,6 +153,7 @@ glamor_put_vbo_space(ScreenPtr screen)
 glBufferData(GL_ARRAY_BUFFER, glamor_priv-vbo_offset,
  glamor_priv-vb, GL_DYNAMIC_DRAW);
 }
+glBindBuffer(GL_ARRAY_BUFFER, 0);

 glBindBuffer(GL_ARRAY_BUFFER, 0);


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

Re: [PATCH 07/10] glamor: Add glamor_program PolyPoint implementation

[Markus Wick]

Am 2014-03-14 07:30, schrieb Keith Packard:

diff --git a/glamor/glamor_polyops.c b/glamor/glamor_polyops.c
index 1484d80..eac1688 100644
--- a/glamor/glamor_polyops.c
+++ b/glamor/glamor_polyops.c
@@ -27,17 +27,93 @@
  */

 #include glamor_priv.h
+#include glamor_transform.h
+
+static const glamor_facet glamor_facet_point = {
+.name = poly_point,
+.vs_vars = attribute vec2 primitive;\n,
+.vs_exec = GLAMOR_POS(gl_Position, primitive),
+};

 static Bool
-_glamor_poly_point(DrawablePtr pDrawable, GCPtr pGC, int mode, int 
npt,

+_glamor_poly_point(DrawablePtr drawable, GCPtr gc, int mode, int npt,
DDXPointPtr ppt, Bool fallback)
 {
-if (!fallback  glamor_ddx_fallback_check_gc(pGC)
- glamor_ddx_fallback_check_pixmap(pDrawable))
+ScreenPtr screen = drawable-pScreen;
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
+glamor_program *prog = glamor_priv-point_prog;
+glamor_pixmap_private *pixmap_priv;
+int nbox = RegionNumRects(gc-pCompositeClip);
+BoxPtr box = RegionRects(gc-pCompositeClip);
+int off_x, off_y;
+GLshort *vbo_ppt;
+char *vbo_offset;
+int box_x, box_y;
+
+if (!fallback  glamor_ddx_fallback_check_gc(gc)
+ glamor_ddx_fallback_check_pixmap(drawable))
 return FALSE;

-miPolyPoint(pDrawable, pGC, mode, npt, ppt);
+pixmap_priv = glamor_get_pixmap_private(pixmap);
+if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
+goto bail;
+
+glamor_get_context(glamor_priv);
+
+if (prog-failed) {
+glamor_put_context(glamor_priv);


Why don't you check this issue before calling glamor_get_context?


+goto bail;
+}
+
+if (!prog-prog) {
+if (!glamor_build_program(screen, prog,
+  glamor_facet_point,
+  glamor_fill_solid))


What do you think about a small API change for glamor_build_program?
imo it would be cleaner to just call glamor_build_program everytime and 
do the fail/successful and already build checks there. Here we don't 
care about everything else but true/false.



+{
+glamor_put_context(glamor_priv);
+goto bail;
+}
+}
+
+if (!glamor_use_program(pixmap, gc, prog)) {
+glamor_put_context(glamor_priv);
+goto bail;
+}
+
+vbo_ppt = glamor_get_vbo_space(screen, npt * (2 * sizeof
(INT16)), vbo_offset);
+glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
+glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_SHORT, GL_FALSE,
0, vbo_offset);
+memcpy(vbo_ppt, ppt, npt * (2 * sizeof (INT16)));
+glamor_put_vbo_space(screen);
+
+glEnable(GL_SCISSOR_TEST);
+
+glamor_pixmap_loop(pixmap_priv, box_x, box_y) {
+glamor_set_destination_drawable(drawable, box_x, box_y, TRUE,
prog-matrix_uniform, off_x, off_y);
+
+while (nbox--) {


How often does this loop iterate typically?
Maybe it's worth to think about a way to also upload all boxes.


+glScissor(box-x1 + off_x,
+  box-y1 + off_y,
+  box-x2 - box-x1,
+  box-y2 - box-y1);
+box++;


I guess box must be reseted in the pixmap_loop.


+glDrawArrays(GL_POINTS, 0, npt);
+}
+}

+glDisable(GL_SCISSOR_TEST);
+glDisableVertexAttribArray(GLAMOR_VERTEX_POS);


We are used to also disable the gl program here unnecessaryly.
I want to strip them all, so I'm fine without it, but it isn't fair for 
a performance comparison.



+
+glamor_put_context(glamor_priv);
+
+glamor_priv-state = RENDER_STATE;
+glamor_priv-render_idle_cnt = 0;
+
+return TRUE;
+
+bail:
+miPolyPoint(drawable, gc, mode, npt, ppt);
 return TRUE;
 }

@@ -55,6 +131,14 @@ glamor_poly_point_nf(DrawablePtr pDrawable, GCPtr
pGC, int mode, int npt,
 return _glamor_poly_point(pDrawable, pGC, mode, npt, ppt, FALSE);
 }

+void
+glamor_fini_point_shader(ScreenPtr screen)
+{
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+
+glamor_delete_program(screen, glamor_priv-point_prog);
+}
+
 static Bool
 _glamor_poly_segment(DrawablePtr pDrawable, GCPtr pGC, int nseg,
  xSegment *pSeg, Bool fallback)

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

Re: [PATCH 09/10] glamor: Add glamor_program based poly_fill_rect

[Markus Wick]

This one looks so similiar with your last one, isn't there a way to 
merge them?
There would be an unneeded multiplication in the shader and the padding 
with one, everything else could be as it is.


So there are also the same issues as in the last patch.
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH 08/10] glamor: Add glamor_program based fill_spans

[Markus Wick]

Am 2014-03-14 07:30, schrieb Keith Packard:

diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
index 96723c0..8c10c92 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -410,7 +410,7 @@ glamor_stipple(PixmapPtr pixmap, PixmapPtr stipple,
 }

 GCOps glamor_gc_ops = {
-.FillSpans = glamor_fill_spans,
+.FillSpans = glamor_fillspans,


This create a lot of dead code. You neither fall back to this old bad
way when glsl130 isn't available nor did you remove it.


 .SetSpans = glamor_set_spans,
 .PutImage = glamor_put_image,
 .CopyArea = glamor_copy_area,
diff --git a/glamor/glamor_spans.c b/glamor/glamor_spans.c
new file mode 100644
index 000..d861232
--- /dev/null
+++ b/glamor/glamor_spans.c
@@ -0,0 +1,127 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
and its
+ * documentation for any purpose is hereby granted without fee,
provided that
+ * the above copyright notice appear in all copies and that both that
copyright
+ * notice and this permission notice appear in supporting
documentation, and
+ * that the name of the copyright holders not be used in advertising
or
+ * publicity pertaining to distribution of the software without
specific,
+ * written prior permission.  The copyright holders make no
representations
+ * about the suitability of this software for any purpose.  It is
provided as
+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL,
INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR
OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#include glamor_priv.h
+#include glamor_transform.h
+
+glamor_program  fill_spans_progs[4];
+
+static const glamor_facet glamor_facet_fillspans = {
+.name = fill_spans,
+.version = 130,
+.vs_vars =  attribute vec3 primitive;\n,
+.vs_exec = (   vec2 pos = vec2(primitive.z,1) *
vec2(gl_VertexID1, (gl_VertexID2)1);\n
+GLAMOR_POS(gl_Position, (primitive.xy + pos))),
+};
+
+void
+glamor_fillspans(DrawablePtr drawable,
+ GCPtr gc,
+ int n, DDXPointPtr points, int *widths, int sorted)
+{
+ScreenPtr screen = drawable-pScreen;
+glamor_screen_private *glamor_priv =
glamor_get_screen_private(screen);
+PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
+glamor_pixmap_private *pixmap_priv;
+glamor_program *prog;
+int nbox = RegionNumRects(gc-pCompositeClip);
+BoxPtr box = RegionRects(gc-pCompositeClip);
+int off_x, off_y;
+GLshort *v;
+char *vbo_offset;
+int c;
+int box_x, box_y;
+
+pixmap_priv = glamor_get_pixmap_private(pixmap);
+if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
+goto bail;
+
+glamor_get_context(glamor_priv);
+
+prog = glamor_use_program_fill(pixmap, gc,
glamor_priv-fill_spans_program,
+   glamor_facet_fillspans);
+
+if (!prog)
+goto bail;
+
+/* Set up the vertex buffers for the points */
+
+v = glamor_get_vbo_space(drawable-pScreen, n * (3 * sizeof
(GLshort)), vbo_offset);
+
+glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
+glVertexAttribDivisor(GLAMOR_VERTEX_POS, 1);


imo we should check for glsl130 support. Just compiling and wait for
errors isn't the way to go.
_but_ this call will crash if ARB_instanced_arrays isn't supported.
As this is a GL3.3 feature, there are likely many plattforms which
doesn't support it.

I don't want to fall back to the old implementation, so I suggest
GL_TRIANGLES + 6 * 2 * sizeof(short) as vertex format.


+glVertexAttribPointer(GLAMOR_VERTEX_POS, 3, GL_SHORT, GL_FALSE,
+  3 * sizeof (GLshort), vbo_offset);


iirc gpus doesn't like vertex strides which aren't multiple of 4 bytes.
Padding with zero?


+
+for (c = 0; c  n; c++) {
+v[0] = points-x;
+v[1] = points-y;
+v[2] = *widths++;
+points++;
+v += 3;
+}
+
+glamor_put_vbo_space(screen);
+
+glEnable(GL_SCISSOR_TEST);
+
+glamor_pixmap_loop(pixmap_priv, box_x, box_y) {
+ glamor_set_destination_drawable(drawable, box_x, box_y,
FALSE, prog-matrix_uniform, off_x, off_y);
+
+ nbox = RegionNumRects(gc-pCompositeClip);
+ box = RegionRects(gc-pCompositeClip);
+
+ while (nbox--) {
+ glScissor(box-x1 + off_x,
+   box-y1 + off_y,
+   box-x2 - box-x1,
+   box-y2 - box-y1);
+ box++;
+ glDrawArraysInstanced(GL_TRIANGLE_STRIP, 0, 4, n);
+ }
+}
+
+glDisable(GL_SCISSOR_TEST);
+   

Re: [PATCH 10/10] glamor: Add glamor_program based poly_text and image_text

[Markus Wick]

Am 2014-03-14 07:30, schrieb Keith Packard:

diff --git a/glamor/glamor_font.c b/glamor/glamor_font.c
new file mode 100644
index 000..4f303b9
--- /dev/null
+++ b/glamor/glamor_font.c
@@ -0,0 +1,181 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software 
and its
+ * documentation for any purpose is hereby granted without fee, 
provided that
+ * the above copyright notice appear in all copies and that both that 
copyright
+ * notice and this permission notice appear in supporting 
documentation, and
+ * that the name of the copyright holders not be used in advertising 
or
+ * publicity pertaining to distribution of the software without 
specific,
+ * written prior permission.  The copyright holders make no 
representations
+ * about the suitability of this software for any purpose.  It is 
provided as

+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS 
SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN 
NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, 
INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS 
OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR 
OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR 
PERFORMANCE

+ * OF THIS SOFTWARE.
+ */
+
+#include glamor_priv.h
+#include glamor_font.h
+#include dixfontstr.h
+
+static int glamor_font_generation;
+static int glamor_font_private_index;
+static int glamor_font_screen_count;
+
+glamor_font_t *
+glamor_font_get(ScreenPtr screen, FontPtr font)
+{
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+
+glamor_font_t   *privates;
+glamor_font_t   *glamor_font;
+int overall_width, overall_height;
+int num_rows;
+int num_cols;
+int glyph_width_pixels;
+int glyph_width_bytes;
+int glyph_height;
+int row, col;
+unsigned char   c[2];
+CharInfoPtr glyph;
+unsigned long   count;
+
+
+privates = FontGetPrivate(font, glamor_font_private_index);
+if (!privates) {
+privates = calloc(glamor_font_screen_count, sizeof 
(glamor_font_t));

+if (!privates)
+return NULL;
+FontSetPrivate(font, glamor_font_private_index, privates);
+}
+
+glamor_font = privates[screen-myNum];
+
+if (glamor_font-realized)
+return glamor_font;
+
+glamor_font-realized = TRUE;
+
+/* Figure out how many glyphs are in the font */
+num_cols = font-info.lastCol - font-info.firstCol + 1;
+num_rows = font-info.lastRow - font-info.firstRow + 1;
+
+/* Figure out the size of each glyph */
+glyph_width_pixels = font-info.maxbounds.rightSideBearing -
font-info.minbounds.leftSideBearing;
+glyph_height = font-info.maxbounds.ascent + 
font-info.maxbounds.descent;

+
+glyph_width_bytes = (glyph_width_pixels + 7)  3;
+
+glamor_font-glyph_width_pixels = glyph_width_pixels;
+glamor_font-glyph_width_bytes = glyph_width_bytes;
+glamor_font-glyph_height = glyph_height;
+
+overall_width = glyph_width_bytes * num_cols;
+overall_height = glyph_height * num_rows;
+
+/* Check whether the font has a default character */
+c[0] = font-info.lastRow + 1;
+c[1] = font-info.lastCol + 1;
+(*font-get_glyphs)(font, 1, c, TwoD16Bit, count, glyph);
+
+glamor_font-has_default = (count != 0);
+
+glamor_priv = glamor_get_screen_private(screen);
+glamor_get_context(glamor_priv);
+
+glGenTextures(1, glamor_font-texture_id);
+glActiveTexture(GL_TEXTURE0);
+glBindTexture(GL_TEXTURE_2D, glamor_font-texture_id);
+
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);


more important to only get one mipmap level: max_level = 0


+
+/* Allocate storage */
+glTexImage2D(GL_TEXTURE_2D, 0, GL_R8UI, overall_width, 
overall_height,

+ 0, GL_RED_INTEGER, GL_UNSIGNED_BYTE, NULL);
+
+glPixelStorei(GL_UNPACK_ALIGNMENT, 4);
+
+/* Paint all of the glyphs */
+for (row = 0; row  num_rows; row++) {
+for (col = 0; col  num_cols; col++) {
+c[0] = row + font-info.firstRow;
+c[1] = col + font-info.firstCol;
+
+(*font-get_glyphs)(font, 1, c, TwoD16Bit, count, 
glyph);

+
+if (count)
+glTexSubImage2D(GL_TEXTURE_2D, 0, col *
glyph_width_bytes, row * glyph_height,
+GLYPHWIDTHBYTES(glyph),
GLYPHHEIGHTPIXELS(glyph),
+GL_RED_INTEGER, GL_UNSIGNED_BYTE, 
glyph-bits);


Here you hope that the driver detects that this texture isn't in use. 
Else you maybe get some kind of async upload / 

Re: [PATCH 06/10] glamor: Add infrastructure for generating shaders on the fly

[Markus Wick]

First, I like to have some common functions to compile our shaders, bind 
common uniforms, defining common headers, ...
But tbh I don't see why it's easier to split up the glsl source 
completely. Most shaders aren't more than 5 loc, so they are easier to 
read entirely.
In dolphin, we've a common glsl header which is prepended for every 
shader and all common functions are defined there. Wouldn't such a 
framework be enough?


Am 2014-03-14 07:29, schrieb Keith Packard:

diff --git a/glamor/glamor_program.c b/glamor/glamor_program.c
new file mode 100644
index 000..b0d4ff0
--- /dev/null
+++ b/glamor/glamor_program.c
@@ -0,0 +1,422 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software 
and its
+ * documentation for any purpose is hereby granted without fee, 
provided that
+ * the above copyright notice appear in all copies and that both that 
copyright
+ * notice and this permission notice appear in supporting 
documentation, and
+ * that the name of the copyright holders not be used in advertising 
or
+ * publicity pertaining to distribution of the software without 
specific,
+ * written prior permission.  The copyright holders make no 
representations
+ * about the suitability of this software for any purpose.  It is 
provided as

+ * is without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS 
SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN 
NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, 
INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS 
OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR 
OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR 
PERFORMANCE

+ * OF THIS SOFTWARE.
+ */
+
+#include glamor_priv.h
+#include glamor_transform.h
+#include glamor_program.h
+
+static Bool
+use_solid (PixmapPtr pixmap, GCPtr gc, glamor_program *prog)
+{
+return glamor_set_solid(pixmap, gc, TRUE, prog-fg_uniform);
+}
+
+const glamor_facet glamor_fill_solid = {
+.name = solid,
+.fs_exec =gl_FragColor = fg;\n,
+.locations = glamor_program_location_fg,
+.use = use_solid,
+};
+
+static Bool
+use_tile (PixmapPtr pixmap, GCPtr gc, glamor_program *prog)
+{
+return glamor_set_tiled(pixmap, gc, prog-fill_offset_uniform,
prog-fill_size_uniform);
+}
+
+static const glamor_facet glamor_fill_tile = {
+.name = tile,
+.version = 130,
+.vs_exec = fill_pos = fill_offset + primitive.xy + 
pos;\n,

+.fs_exec = gl_FragColor = texelFetch(sampler,
ivec2(mod(fill_pos,fill_size)), 0);\n,
+.locations = glamor_program_location_fill,
+.use = use_tile,
+};
+
+#if 0
+static Bool
+use_stipple (PixmapPtr pixmap, GCPtr gc, glamor_program *prog)
+{
+return glamor_set_stippled(pixmap, gc, prog-fg_uniform,
prog-fill_offset_uniform, prog-fill_size_uniform);
+}
+
+static const glamor_facet glamor_fill_stipple = {
+.name = stipple,
+.version = 130,
+.vs_exec = fill_pos = fill_offset + primitive.xy + 
pos;\n;

+.fs_exec = (   if (texelFetch(sampler,
ivec2(mod(fill_pos,fill_size)), 0).x == 0)\n
+   discard;\n
+   gl_FragColor = fg;\n)
+.locations = glamor_program_location_fg | 
glamor_program_location_fill

+.use = use_stipple,
+};
+
+static const glamor_facet glamor_fill_opaque_stipple = {
+.name = opaque_stipple,
+.version = 130,
+.vs_exec = fill_pos = fill_offset + primitive.xy + 
pos;\n;

+.fs_exec = (   if (texelFetch(sampler,
ivec2(mod(fill_pos,fill_size)), 0).x == 0)\n
+   gl_FragColor = bg;\n
+   else\n
+   gl_FragColor = fg;\n),
+.locations = glamor_program_location_fg |
glamor_program_location_bg | glamor_program_location_fill
+.use = use_opaque_stipple
+};
+#endif
+
+static const glamor_facet *glamor_facet_fill[4] = {
+glamor_fill_solid,
+glamor_fill_tile,
+NULL,
+NULL,
+};
+
+typedef struct {
+glamor_program_location location;
+const char  *vs_vars;
+const char  *fs_vars;
+} glamor_location_var;
+
+static glamor_location_var location_vars[] = {
+{
+.location = glamor_program_location_fg,
+.fs_vars = uniform vec4 fg;\n
+},
+{
+.location = glamor_program_location_bg,
+.fs_vars = uniform vec4 bg;\n
+},
+{
+.location = glamor_program_location_fill,
+.vs_vars = (uniform vec2 fill_offset;\n
+varying vec2 fill_pos;\n),
+.fs_vars = (uniform sampler2D sampler;\n
+uniform vec2 fill_size;\n
+varying vec2 fill_pos;\n)
+},
+{
+.location = glamor_program_location_font,
+.fs_vars = uniform usampler2D font;\n,
+},

Re: [PATCH 00/10] First Glamor core acceleration attempt

[Markus Wick]

Patch 1 seems to fix my issue on nvidia, so:
Tested-by: Markus Wick mar...@selfnet.de

Patches 2-5 are:
Reviewed-by: Markus Wick mar...@selfnet.de

Patches 7-9 are really nice, but there should be a glsl130+instancing 
fallback with another vertex format + shaders.


Patch 10 also have to check for gl extensions. We could fall back to r8 
textures, but the mi fallback also sounds fine, so:

Reviewed-by: Markus Wick mar...@selfnet.de

But I get some broken renderings in xterm on nvidia because of the last 
patch. I guess it's because of the 0.5 offset.

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

Re: [PATCH 05/27] glamor: Do glyph private init at screeninit time, and other stuff at CSR.

[Markus Wick]

Am 2014-03-11 22:30, schrieb Eric Anholt:

diff --git a/glamor/glamor.h b/glamor/glamor.h
index e25dc73..9cda46d 100644
--- a/glamor/glamor.h
+++ b/glamor/glamor.h
@@ -131,14 +131,6 @@ extern _X_EXPORT void
glamor_set_screen_pixmap(PixmapPtr screen_pixmap,

 extern _X_EXPORT uint32_t glamor_get_pixmap_texture(PixmapPtr pixmap);

-/* @glamor_glyphs_init: Initialize glyphs internal data structures.
- *
- * @pScreen: Current screen pointer.
- *
- * This function must be called after the glamor_init and the texture
- * can be allocated. An example is to call it when create the screen
- * resources at DDX layer.
- */
 extern _X_EXPORT Bool glamor_glyphs_init(ScreenPtr pScreen);


This function isn't (and shouldn't be?) used outside of glamor, so this 
should be moved to glamor_priv.h.




 extern _X_EXPORT void glamor_set_pixmap_texture(PixmapPtr pixmap,
diff --git a/glamor/glamor_glyphs.c b/glamor/glamor_glyphs.c
index caafa43..2b2c735 100644
--- a/glamor/glamor_glyphs.c
+++ b/glamor/glamor_glyphs.c
@@ -370,16 +366,27 @@ glamor_realize_glyph_caches(ScreenPtr pScreen)
 return FALSE;
 }

+/**
+ * Called by glamor_create_screen_resources() to set up the glyph 
cache.

+ *
+ * This was previously required to be called by the drivers, but not
+ * as of the xserver 1.16 ABI.
+ */
 Bool
 glamor_glyphs_init(ScreenPtr pScreen)
 {
+glamor_screen_private *glamor = 
glamor_get_screen_private(pScreen);

+
+if (glamor-glyph_cache_initialized)
+return TRUE;


Is it still possible that this function is called twice? I don't think 
so, so glyph_cache_initialized isn't needed any more.



+
 if (!dixRegisterPrivateKey(glamor_glyph_key,
PRIVATE_GLYPH, sizeof(struct 
glamor_glyph)))

 return FALSE;

-/* Skip pixmap creation if we don't intend to use it. */
+glamor-glyph_cache_initialized = TRUE;

-return glamor_realize_glyph_caches(pScreen);
+return TRUE;
 }

 /* The most efficient thing to way to upload the glyph to the screen

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

Re: [PATCH 16/27] glamor: Replace some goofy enum-likes with a real enum.

[Markus Wick]

Am 2014-03-11 22:30, schrieb Eric Anholt:

This unpacks the bitfield into an int size, but my experience has been
that packing bitfields doesn't matter for performance.

Signed-off-by: Eric Anholt e...@anholt.net
---
 glamor/glamor_fbo.c  |  2 +-
 glamor/glamor_priv.h | 25 -
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/glamor/glamor_fbo.c b/glamor/glamor_fbo.c
index 281cf83..640b6fd 100644
--- a/glamor/glamor_fbo.c
+++ b/glamor/glamor_fbo.c
@@ -505,7 +505,7 @@ glamor_pixmap_attach_fbo(PixmapPtr pixmap,
glamor_pixmap_fbo *fbo)
 case GLAMOR_TEXTURE_LARGE:
 case GLAMOR_TEXTURE_ONLY:
 case GLAMOR_TEXTURE_DRM:
-pixmap_priv-base.gl_fbo = 1;
+pixmap_priv-base.gl_fbo = GLAMOR_FBO_NORMAL;
 if (fbo-tex != 0)
 pixmap_priv-base.gl_tex = 1;
 else {
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 4dc2c75..52dad35 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -291,8 +291,21 @@ typedef enum glamor_access {
 GLAMOR_ACCESS_WO,
 } glamor_access_t;

-#define GLAMOR_FBO_NORMAL 1
-#define GLAMOR_FBO_DOWNLOADED 2
+enum glamor_fbo_state {
+/** There is no storage attached to the pixmap. */
+GLAMOR_FBO_UNATTACHED,
+/**
+ * The pixmap has FBO storage attached, but devPrivate.ptr doesn't
+ * point at anything.
+ */
+GLAMOR_FBO_NORMAL,
+/**
+ * The FBO is present and can be accessed as a linear memory
+ * mapping through devPrivate.ptr.
+ */
+GLAMOR_FBO_DOWNLOADED,
+};
+
 /* glamor_pixmap_fbo:
  * @list:to be used to link to the cache pool list.
  * @expire:  when push to cache pool list, set a expire count.
@@ -324,12 +337,6 @@ typedef struct glamor_pixmap_fbo {

 /*
  * glamor_pixmap_private - glamor pixmap's private structure.
- * @gl_fbo:
- * 0   - The pixmap doesn't has a fbo attached to it.
- * 	GLAMOR_FBO_NORMAL 	- The pixmap has a fbo and can be accessed 
normally.
- * 	GLAMOR_FBO_DOWNLOADED 	- The pixmap has a fbo and already 
downloaded to

- *   CPU, so it can only be treated as a in-memory 
pixmap
- *   if this bit is set.
  * @gl_tex:  The pixmap is in a gl texture originally.
  * @is_picture: The drawable is attached to a picture.
  * @pict_format: the corresponding picture's format.
@@ -403,7 +410,7 @@ typedef struct glamor_pixmap_clipped_regions {

 typedef struct glamor_pixmap_private_base {
 glamor_pixmap_type_t type;
-unsigned char gl_fbo:2;
+enum glamor_fbo_state gl_fbo;
 unsigned char is_picture:1;
 unsigned char gl_tex:1;
 glamor_pixmap_fbo *fbo;


base.gl_fbo seems often to be compared vs 0 instead of 
GLAMOR_FBO_UNATTACHED.


To be honest, I'm a bit confused about this flag. Is this really about 
fbo or more likely about general textures?

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

Re: [PATCH 18/27] glamor: Allow nested mapping of pixmaps.

[Markus Wick]

Am 2014-03-11 22:30, schrieb Eric Anholt:

The common pattern is to do nested if statements making calls to
prepare_access() and then pop those mappings back off in each set of
braces.  Some cases checked for src == dst to avoid leaking mappings,
but others didn't.  Others didn't even do the nested mappings, so a
failure in the outer map would result in trying to umap the inner and
failing.

By allowing nested mappings, we can fix both problems by not requiring
the care from the caller, plus we can allow a simpler nesting of all
the prepares in one if statement.

Signed-off-by: Eric Anholt e...@anholt.net
---
 glamor/glamor_core.c | 19 ++-
 glamor/glamor_priv.h |  6 ++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
index 5883809..7a7ca08 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -105,6 +105,19 @@ Bool
 glamor_prepare_access(DrawablePtr drawable, glamor_access_t access)
 {
 PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
+glamor_pixmap_private *pixmap_priv = 
glamor_get_pixmap_private(pixmap);

+
+if (pixmap-devPrivate.ptr) {
+/* Already mapped, nothing needs to be done.  Note that we
+ * aren't allowing promotion from RO to RW, because it would
+ * require re-mapping the PBO.
+ */
+assert(!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv) ||
+   access == GLAMOR_ACCESS_RO ||
+   pixmap_priv-base.mapped_for_write);
+return TRUE;
+}
+pixmap_priv-base.mapped_for_write = (access == GLAMOR_ACCESS_RW);

 return glamor_download_pixmap_to_cpu(pixmap, access);
 }
@@ -300,7 +313,11 @@ glamor_finish_access(DrawablePtr drawable,
glamor_access_t access_mode)
 if (!GLAMOR_PIXMAP_PRIV_HAS_FBO_DOWNLOADED(pixmap_priv))
 return;

-if (access_mode != GLAMOR_ACCESS_RO) {
+/* If we are doing a series of unmaps from a nested map, we're 
done. */

+if (!pixmap-devPrivate.ptr)
+return;


In my opinion, there should be a note that this will unmap on the 
innermost call to finish_access, but the outer functions maybe still 
want to access this pixmap.



+
+if (pixmap_priv-base.mapped_for_write) {
 glamor_restore_pixmap_to_texture(pixmap);
 }

diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 7f41025..24a3575 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -410,6 +410,12 @@ typedef struct glamor_pixmap_clipped_regions {
 typedef struct glamor_pixmap_private_base {
 glamor_pixmap_type_t type;
 enum glamor_fbo_state gl_fbo;
+/**
+ * If devPrivate.ptr is non-NULL (meaning we're within
+ * glamor_prepare_access), determies whether we should re-upload
+ * that data on glamor_finish_access().
+ */
+bool mapped_for_write;


Why haven't you used the enum glamor_access? Memory footprint shouldn't 
matter that much.



 unsigned char is_picture:1;
 unsigned char gl_tex:1;
 glamor_pixmap_fbo *fbo;

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

Re: [PATCH 21/27] glamor: Improve the performance of PushPixels by, well, pushing pixels.

[Markus Wick]

I didn't find any issues in this patch, but I don't see the point:
Do we really want to convert a pixmap with 1 bit per pixel into a list 
of enabled pixels on cpu?
Isn't it as easy to upload this pixmap as texture and do everything else 
in the pixel shader?

In this way, we won't have to care about the location of the bitmap.

Am 2014-03-11 22:30, schrieb Eric Anholt:

Otherwise, mi will fall back to GetSpans()ing the bitmap, walking the
bitmap, computing spans to be filled, and calling FillSpans().

Improves x11perf -f8text by 759.373% +/- 3.33096% (n=166)

Signed-off-by: Eric Anholt e...@anholt.net
---
 glamor/glamor_glyphblt.c | 115 
+++

 1 file changed, 115 insertions(+)

diff --git a/glamor/glamor_glyphblt.c b/glamor/glamor_glyphblt.c
index 6f754ce..0a99a95 100644
--- a/glamor/glamor_glyphblt.c
+++ b/glamor/glamor_glyphblt.c
@@ -91,15 +91,130 @@ glamor_poly_glyph_blt_nf(DrawablePtr pDrawable, 
GCPtr pGC,

 }

 static Bool
+glamor_push_pixels_points(GCPtr gc, PixmapPtr bitmap,
+  DrawablePtr drawable, int w, int h, int x, 
int y)

+{
+ScreenPtr screen = drawable-pScreen;
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
+glamor_pixmap_private *pixmap_priv;
+uint8_t *bitmap_data = bitmap-devPrivate.ptr;
+int bitmap_stride = bitmap-devKind;
+int off_x, off_y;
+int yy, xx;
+GLfloat xscale, yscale;
+float color[4];
+unsigned long fg_pixel = gc-fgPixel;
+float *points, *next_point;
+int num_points = 0;
+char *vbo_offset;
+RegionPtr clip;
+
+if (w * h  MAXINT / (2 * sizeof(float)))
+return FALSE;
+
+if (gc-fillStyle != FillSolid) {
+glamor_fallback(gc fillstyle not solid\n);
+return FALSE;
+}
+
+pixmap_priv = glamor_get_pixmap_private(pixmap);
+if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
+return FALSE;
+
+glamor_get_context(glamor_priv);
+if (!glamor_set_alu(screen, gc-alu)) {
+if (gc-alu == GXclear)
+fg_pixel = 0;
+else {
+glamor_fallback(unsupported alu %x\n, gc-alu);
+glamor_put_context(glamor_priv);
+return FALSE;
+}
+}
+
+if (!glamor_set_planemask(pixmap, gc-planemask)) {
+glamor_fallback(Failed to set planemask in %s.\n, 
__FUNCTION__);

+glamor_put_context(glamor_priv);
+return FALSE;
+}
+
+glamor_get_drawable_deltas(drawable, pixmap, off_x, off_y);
+
+glamor_set_destination_pixmap_priv_nc(pixmap_priv);
+pixmap_priv_get_dest_scale(pixmap_priv, xscale, yscale);
+
+glUseProgram(glamor_priv-solid_prog);
+
+glamor_get_rgba_from_pixel(fg_pixel,
+   color[0], color[1], color[2], 
color[3],

+   format_for_pixmap(pixmap));
+glUniform4fv(glamor_priv-solid_color_uniform_location, 1, color);
+
+points = glamor_get_vbo_space(screen, w * h * sizeof(float) * 2,
+  vbo_offset);
+next_point = points;
+
+clip = fbGetCompositeClip(gc);
+
+/* Note that because fb sets miTranslate in the GC, our incoming X
+ * and Y are in screen coordinate space (same for spans, but not
+ * other operations).
+ */
+for (yy = 0; yy  h; yy++) {
+uint8_t *bitmap_row = bitmap_data + yy * bitmap_stride;
+for (xx = 0; xx  w; xx++) {
+if (bitmap_row[xx / 8]  (1  xx % 8) 
+RegionContainsPoint(clip,
+x + xx,
+y + yy,
+NULL)) {
+next_point[0] = v_from_x_coord_x(xscale, x + xx + 
off_x + 0.5);

+if (glamor_priv-yInverted)
+next_point[1] = v_from_x_coord_y_inverted(yscale,
y + yy + off_y + 0.5);
+else
+next_point[1] = v_from_x_coord_y(yscale, y + yy +
off_y + 0.5);
+
+next_point += 2;
+num_points++;
+}
+}
+}
+glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_FLOAT,
+  GL_FALSE, 2 * sizeof(float),
+  vbo_offset);
+glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
+
+glamor_put_vbo_space(screen);
+
+glDrawArrays(GL_POINTS, 0, num_points);
+
+glDisableVertexAttribArray(GLAMOR_VERTEX_POS);
+glUseProgram(0);
+
+glamor_put_context(glamor_priv);
+
+return TRUE;
+}
+
+static Bool
 _glamor_push_pixels(GCPtr pGC, PixmapPtr pBitmap,
 DrawablePtr pDrawable, int w, int h, int x, int y,
 Bool fallback)
 {
+glamor_pixmap_private *pixmap_priv;
+
 if (!fallback  glamor_ddx_fallback_check_pixmap(pDrawable)
  glamor_ddx_fallback_check_pixmap(pBitmap-drawable)
  glamor_ddx_fallback_check_gc(pGC))
 return FALSE;

+  

Re: [PATCH 22/27] glamor: Improve the performance of PolyGlyphBlt.

[Markus Wick]

Are we able to add cached textures to CharInfoPtr? If not, then this 
would be the ideal use case for texture_arrays ;)
Again, using GL_POINTS and check for every bit on CPU isn't the way to 
go.


Am 2014-03-11 22:30, schrieb Eric Anholt:

Using the same idea as the previous PushPixels code, just make points
for each point in the glyph.  This is an advantage over the pushpixels
fallback because we can batch the BO mappings and draw calls across
glyphs.

Improves performance of x11perf -f8text by 773.389% +/- 3.50754% 
(n=10).


Signed-off-by: Eric Anholt e...@anholt.net
---
 glamor/glamor_glyphblt.c | 138 
+++

 1 file changed, 138 insertions(+)

diff --git a/glamor/glamor_glyphblt.c b/glamor/glamor_glyphblt.c
index 0a99a95..5d785a0 100644
--- a/glamor/glamor_glyphblt.c
+++ b/glamor/glamor_glyphblt.c
@@ -27,6 +27,141 @@
  */

 #include glamor_priv.h
+#include dixfontstr.h
+
+static Bool
+glamor_poly_glyph_blt_pixels(DrawablePtr drawable, GCPtr gc,
+ int x, int y, unsigned int nglyph,
+ CharInfoPtr *ppci)
+{
+ScreenPtr screen = drawable-pScreen;
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
+glamor_pixmap_private *pixmap_priv;
+int off_x, off_y;
+GLfloat xscale, yscale;
+float color[4];
+unsigned long fg_pixel = gc-fgPixel;
+char *vbo_offset;
+RegionPtr clip;
+int num_points, max_points;
+float *points = NULL;
+
+x += drawable-x;
+y += drawable-y;
+
+if (gc-fillStyle != FillSolid) {
+glamor_fallback(gc fillstyle not solid\n);
+return FALSE;
+}
+
+pixmap_priv = glamor_get_pixmap_private(pixmap);
+if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv))
+return FALSE;
+
+glamor_get_context(glamor_priv);
+if (!glamor_set_alu(screen, gc-alu)) {
+if (gc-alu == GXclear)
+fg_pixel = 0;
+else {
+glamor_fallback(unsupported alu %x\n, gc-alu);
+glamor_put_context(glamor_priv);
+return FALSE;
+}
+}
+
+if (!glamor_set_planemask(pixmap, gc-planemask)) {
+glamor_fallback(Failed to set planemask in %s.\n, 
__FUNCTION__);

+glamor_put_context(glamor_priv);
+return FALSE;
+}
+
+glamor_get_drawable_deltas(drawable, pixmap, off_x, off_y);
+
+glamor_set_destination_pixmap_priv_nc(pixmap_priv);
+pixmap_priv_get_dest_scale(pixmap_priv, xscale, yscale);
+
+glUseProgram(glamor_priv-solid_prog);
+
+glamor_get_rgba_from_pixel(fg_pixel,
+   color[0], color[1], color[2], 
color[3],

+   format_for_pixmap(pixmap));
+glUniform4fv(glamor_priv-solid_color_uniform_location, 1, color);
+
+clip = fbGetCompositeClip(gc);
+
+glEnableVertexAttribArray(GLAMOR_VERTEX_POS);
+
+max_points = 500;
+num_points = 0;
+while (nglyph--) {
+CharInfoPtr charinfo = *ppci++;
+int w = GLYPHWIDTHPIXELS(charinfo);
+int h = GLYPHHEIGHTPIXELS(charinfo);
+uint8_t *glyphbits = FONTGLYPHBITS(NULL, charinfo);
+
+if (w  h) {
+int glyph_x = x + charinfo-metrics.leftSideBearing;
+int glyph_y = y - charinfo-metrics.ascent;
+int glyph_stride = GLYPHWIDTHBYTESPADDED(charinfo);
+int xx, yy;
+
+for (yy = 0; yy  h; yy++) {
+uint8_t *glyph_row = glyphbits + glyph_stride * yy;
+for (xx = 0; xx  w; xx++) {
+int pt_x_i = glyph_x + xx;
+int pt_y_i = glyph_y + yy;
+float pt_x_f, pt_y_f;
+if (!(glyph_row[xx / 8]  (1  xx % 8)))
+continue;
+
+if (!RegionContainsPoint(clip, pt_x_i, pt_y_i, 
NULL))

+continue;
+
+if (!num_points) {
+points = glamor_get_vbo_space(screen,
+  max_points * 2
* sizeof(float),
+  vbo_offset);
+
+glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, 
GL_FLOAT,
+  GL_FALSE, 2 * 
sizeof(float),

+  vbo_offset);
+}
+
+pt_x_f = v_from_x_coord_x(xscale, pt_x_i + off_x + 
0.5);

+if (glamor_priv-yInverted)
+pt_y_f = v_from_x_coord_y_inverted(yscale,
pt_y_i + off_y + 0.5);
+else
+pt_y_f = v_from_x_coord_y(yscale, pt_y_i +
off_y + 0.5);
+
+points[num_points * 2 + 0] = pt_x_f;
+points[num_points * 2 + 1] = pt_y_f;
+num_points++;
+
+if (num_points == max_points) {
+  

Re: [PATCH 23/27] glamor: Improve the performance of line fallbacks.

[Markus Wick]

Is there a generic mi fallback for all poly_lines? If not, I'd suggest 
to move this logic into mi itself.


Am 2014-03-11 22:30, schrieb Eric Anholt:

If the lines aren't solid-filled vert/horiz solid-filled rectangles,
we fall back.  libreoffice has some diagonal lines, and the
performance of the fallback path was atrocious.  Just fall back to
mi's spans instead, so that we don't do an upload/download.

Improves x11perf -seg100 by 863.652% +/- 9.8968% (n=5)

Signed-off-by: Eric Anholt e...@anholt.net
---
 glamor/glamor_polylines.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/glamor/glamor_polylines.c b/glamor/glamor_polylines.c
index 697fc9e..1adf45d 100644
--- a/glamor/glamor_polylines.c
+++ b/glamor/glamor_polylines.c
@@ -51,8 +51,9 @@ _glamor_poly_lines(DrawablePtr drawable, GCPtr gc,
int mode, int n,
 /* This ends up in miSetSpans, which is accelerated as well as 
we

  * can hope X wide lines will be.
  */
-goto wide_line;
+goto fail;
 }
+
 if (gc-lineStyle != LineSolid) {
 glamor_fallback(non-solid fill line style %d\n, 
gc-lineStyle);

 goto fail;
@@ -104,19 +105,19 @@ _glamor_poly_lines(DrawablePtr drawable, GCPtr
gc, int mode, int n,
  glamor_ddx_fallback_check_gc(gc))
 return FALSE;

-if (gc-lineWidth == 0) {
-if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW) 
-glamor_prepare_access_gc(gc)) {
-fbPolyLine(drawable, gc, mode, n, points);
-}
-glamor_finish_access_gc(gc);
-glamor_finish_access(drawable);
-}
-else {
- wide_line:
-/* fb calls mi functions in the lineWidth != 0 case. */
-fbPolyLine(drawable, gc, mode, n, points);
+switch (gc-lineStyle) {
+case LineSolid:
+if (gc-lineWidth == 0)
+miZeroLine(drawable, gc, mode, n, points);
+else
+miWideLine(drawable, gc, mode, n, points);
+break;
+case LineOnOffDash:
+case LineDoubleDash:
+miWideDash(drawable, gc, mode, n, points);
+break;
 }
+
 return TRUE;
 }

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

Re: glamor-server subset patch bomb

[Markus Wick]

wow, big patch series ...

Patches 3-5, 7-22, 24-27 are:
Reviewed-by: Markus Wick mar...@selfnet.de

But as 21-22 are very big one, I'd like to see another review from 
someone familiar with the X11 renderings. But the gl code is fine :D


Patches 1, 2, 6, 23 also looks fine for me, but I don't know anything 
about this subsystem.


I'm still a bit in worry about 21-22 as they should be implemented in a 
completely different way imo using textures. But this would require 
glsl130 because of integers.


Am 2014-03-11 22:30, schrieb Eric Anholt:

keithp said he was tired of the glamor stuff trickling in and wanted
to just review it all at once and be done.  I know I hate reviewing
giant patch series, but who am I to argue with someone who says they
want to do more review?

This series fixes a bunch of CopyPlane XTS tests in Xephyr, thanks to
the GC prepare fixes if I remember right.

There are a few patches that used to be in glamor-server that I
dropped.  Some were performance ideas that didn't produce measurable
results (yet -- in one case, it's that I think I misplaced a spans
patch I used to have, which was a prereq for another change being
useful).  Others just regressed things.  So, if you've got your own
glamor branches based off of my glamor-server, be careful not to carry
that junk around.

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

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

Re: [PATCH 8/9] glamor: Fix requested composite VBO size.

[Markus Wick]

Am 2014-03-09 05:07, schrieb Eric Anholt:

The argument to setup_composte_vbo is the number of verts.

Signed-off-by: Eric Anholt e...@anholt.net
---
 glamor/glamor_render.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index 7829977..63bddfd 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -1253,8 +1253,7 @@ glamor_composite_with_shader(CARD8 op,
 vert_stride += 4;
 }

-nrect_max = (vert_stride * nrect)  GLAMOR_COMPOSITE_VBO_VERT_CNT 
?

-(GLAMOR_COMPOSITE_VBO_VERT_CNT / vert_stride) : nrect;
+nrect_max = MIN(nrect, GLAMOR_COMPOSITE_VBO_VERT_CNT / 4);

 while (nrect) {
 int mrect, rect_processed;
@@ -1262,7 +1261,7 @@ glamor_composite_with_shader(CARD8 op,
 float *vertices;

 mrect = nrect  nrect_max ? nrect_max : nrect;
-vertices = glamor_setup_composite_vbo(screen, mrect * 
vert_stride);

+vertices = glamor_setup_composite_vbo(screen, mrect * 4);
 rect_processed = mrect;
 vb_stride = glamor_priv-vb_stride / sizeof(float);
 while (mrect--) {


vert_stride isn't accessed any more at all. I think this is fine as also 
vb_stride is build in the same way. But do you want to remove 
vert_stride?

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

Re: [PATCH 9/9] glamor: Use buffer_storage

[Markus Wick]

Am 2014-03-09 05:07, schrieb Eric Anholt:

v2:
  - Make the default buffer size a #define. (by Markus Wick)
  - Fix the return offset for mapping with buffer_storage.  (oops!)
v3:
  - Avoid GL error at first rendering from unmapping no buffer.
  - Rebase on the glBindBuffer(GL_ARRAY_BUFFER, 0) change.
v4: Rebase on Markus's vbo init changes.

Signed-off-by: Eric Anholt e...@anholt.net
---
 glamor/glamor.c  |  2 ++
 glamor/glamor_priv.h |  1 +
 glamor/glamor_vbo.c  | 51 
+--

 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/glamor/glamor.c b/glamor/glamor.c
index dc69c72..e856179 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -353,6 +353,8 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 glamor_gl_has_extension(GL_MESA_pack_invert);
 glamor_priv-has_fbo_blit =
 glamor_gl_has_extension(GL_EXT_framebuffer_blit);
+glamor_priv-has_buffer_storage =
+glamor_gl_has_extension(GL_ARB_buffer_storage);
 glGetIntegerv(GL_MAX_RENDERBUFFER_SIZE, 
glamor_priv-max_fbo_size);

 #ifdef MAX_FBO_SIZE
 glamor_priv-max_fbo_size = MAX_FBO_SIZE;
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 986e729..d15eabd 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -208,6 +208,7 @@ typedef struct glamor_screen_private {
 enum glamor_gl_flavor gl_flavor;
 int has_pack_invert;
 int has_fbo_blit;
+int has_buffer_storage;
 int max_fbo_size;

 struct xorg_list
diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
index be2c2af..f736cbe 100644
--- a/glamor/glamor_vbo.c
+++ b/glamor/glamor_vbo.c
@@ -52,7 +52,49 @@ glamor_get_vbo_space(ScreenPtr screen, unsigned
size, char **vbo_offset)

 glBindBuffer(GL_ARRAY_BUFFER, glamor_priv-vbo);

-if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
+if (glamor_priv-has_buffer_storage) {
+if (glamor_priv-vbo_size  glamor_priv-vbo_offset + size) {
+if (glamor_priv-vbo_size)
+glUnmapBuffer(GL_ARRAY_BUFFER);
+
+if (size  glamor_priv-vbo_size) {
+glamor_priv-vbo_size = MAX(GLAMOR_VBO_SIZE, size);
+
+/* We aren't allowed to resize glBufferStorage()
+ * buffers, so we need to gen a new one.
+ */
+glDeleteBuffers(1, glamor_priv-vbo);
+glGenBuffers(1, glamor_priv-vbo);
+glBindBuffer(GL_ARRAY_BUFFER, glamor_priv-vbo);
+
+assert(glGetError() == GL_NO_ERROR);


On release builds, glGetError won't be called and so this won't clear 
the gl error log. So we'll fall back to the mbr code because of any gl 
error somewhere in glamor.


+glBufferStorage(GL_ARRAY_BUFFER, 
glamor_priv-vbo_size, NULL,

+GL_MAP_WRITE_BIT |
+GL_MAP_PERSISTENT_BIT |
+GL_MAP_COHERENT_BIT);
+
+if (glGetError() != GL_NO_ERROR) {
+/* If the driver failed our coherent mapping, fall
+ * back to the ARB_mbr path.
+ */
+glamor_priv-has_buffer_storage = false;
+glamor_priv-vbo_size = 0;


glamor_put_context(glamor_priv);

+return glamor_get_vbo_space(screen, size, 
vbo_offset);

+}
+}
+
+glamor_priv-vbo_offset = 0;
+glamor_priv-vb = glMapBufferRange(GL_ARRAY_BUFFER,
+   0, 
glamor_priv-vbo_size,

+   GL_MAP_WRITE_BIT |
+   
GL_MAP_INVALIDATE_BUFFER_BIT |

+   GL_MAP_PERSISTENT_BIT |
+   GL_MAP_COHERENT_BIT);
+}
+*vbo_offset = (void *)(uintptr_t)glamor_priv-vbo_offset;
+data = glamor_priv-vb + glamor_priv-vbo_offset;
+glamor_priv-vbo_offset += size;
+} else if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
 if (glamor_priv-vbo_size  glamor_priv-vbo_offset + size) {
 glamor_priv-vbo_size = MAX(GLAMOR_VBO_SIZE, size);
 glamor_priv-vbo_offset = 0;
@@ -99,7 +141,12 @@ glamor_put_vbo_space(ScreenPtr screen)

 glamor_get_context(glamor_priv);

-if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
+if (glamor_priv-has_buffer_storage) {
+/* If we're in the ARB_buffer_storage path, we have a
+ * persistent mapping, so we can leave it around until we
+ * reach the end of the buffer.
+ */
+} else if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
 glUnmapBuffer(GL_ARRAY_BUFFER);
 } else {
 glBufferData(GL_ARRAY_BUFFER, glamor_priv-vbo_offset,


Which kind of error do you expect for a driver which isn't able to map 
coherent? I've only found a GL_OUT_OF_MEMORY

Re: [PATCH 5/9] glamor: Track the next vertex offset as we go for non-AA traps.

[Markus Wick]

Am 2014-03-09 05:07, schrieb Eric Anholt:

+vb += i * glamor_priv-vb_stride / 4;

This should be / sizeof(float) instead.



+vb += 3 * glamor_priv-vb_stride / 4;

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

Re: [PATCH 7/9] glamor: Extract the streamed vertex data code used by Render.

[Markus Wick]

Am 2014-03-09 05:07, schrieb Eric Anholt:

diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
new file mode 100644
index 000..be2c2af
--- /dev/null
+++ b/glamor/glamor_vbo.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person 
obtaining a
+ * copy of this software and associated documentation files (the 
Software),
+ * to deal in the Software without restriction, including without 
limitation
+ * the rights to use, copy, modify, merge, publish, distribute, 
sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom 
the
+ * Software is furnished to do so, subject to the following 
conditions:

+ *
+ * The above copyright notice and this permission notice (including 
the next
+ * paragraph) shall be included in all copies or substantial portions 
of the

+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, 
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
DEALINGS

+ * IN THE SOFTWARE.
+ */
+
+/**
+ * @file glamor_vbo.c
+ *
+ * Helpers for managing streamed vertex bufffers used in glamor.
+ */
+
+#include glamor_priv.h
+
+/** Default size of the VBO, in bytes.
+ *
+ * If a single request is larger than this size, we'll resize the VBO
+ * and return an appropriate mapping, but we'll resize back down after
+ * that to avoid hogging that memory forever.  We don't anticipate
+ * normal usage actually requiring larger VBO sizes.
+ */
+#define GLAMOR_VBO_SIZE (64 * 1024)


This is a bit too small imo. iirc glamor was used to split up renderings 
to 64k vertices, not 64k bytes.
What is the cache implact on too big buffers? i965 must fall through to 
LLC, so will it pollute the L1+L2 caches?
For non-coherent gpus, write combining also shouldn't pollute any 
caches.



+
+/**
+ * Returns a pointer to @size bytes of VBO storage, which should be
+ * accessed by the GL using vbo_offset within the VBO.
+ */
+void *
+glamor_get_vbo_space(ScreenPtr screen, unsigned size, char 
**vbo_offset)

+{
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+void *data;
+
+glamor_get_context(glamor_priv);
+
+glBindBuffer(GL_ARRAY_BUFFER, glamor_priv-vbo);
+
+if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
+if (glamor_priv-vbo_size  glamor_priv-vbo_offset + size) {
+glamor_priv-vbo_size = MAX(GLAMOR_VBO_SIZE, size);
+glamor_priv-vbo_offset = 0;
+glBufferData(GL_ARRAY_BUFFER,
+ glamor_priv-vbo_size, NULL, GL_STREAM_DRAW);
+}
+
+data = glMapBufferRange(GL_ARRAY_BUFFER,
+glamor_priv-vbo_offset,
+size,
+GL_MAP_WRITE_BIT |
+GL_MAP_UNSYNCHRONIZED_BIT |
+GL_MAP_INVALIDATE_RANGE_BIT);
+assert(data != NULL);
+*vbo_offset = (void *)(uintptr_t)glamor_priv-vbo_offset;


(char *) instead of (void *).


+glamor_priv-vbo_offset += size;
+} else {
+/* Return a pointer to the statically allocated non-VBO
+ * memory. We'll upload it through glBufferData() later.
+ */
+if (glamor_priv-vbo_size  size) {
+glamor_priv-vbo_size = MAX(GLAMOR_VBO_SIZE, size);
+
+glBufferData(GL_ARRAY_BUFFER,
+ glamor_priv-vbo_size, NULL, GL_STREAM_DRAW);


Is this call needed at all? As we upload by glBufferData, we will alloc 
the buffer then.



+
+free(glamor_priv-vb);
+glamor_priv-vb = XNFalloc(size);
+}
+*vbo_offset = NULL;
+glamor_priv-vbo_offset = 0;


This variable is used for the size on uploading. So it must be set to 
size.



+data = glamor_priv-vb;
+}
+
+glamor_put_context(glamor_priv);
+
+return data;
+}
+
+void
+glamor_put_vbo_space(ScreenPtr screen)
+{
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+
+glamor_get_context(glamor_priv);
+
+if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
+glUnmapBuffer(GL_ARRAY_BUFFER);
+} else {
+glBufferData(GL_ARRAY_BUFFER, glamor_priv-vbo_offset,
+ glamor_priv-vb, GL_DYNAMIC_DRAW);
+}
+
+glBindBuffer(GL_ARRAY_BUFFER, 0);
+
+glamor_put_context(glamor_priv);
+}
+
+void
+glamor_init_vbo(ScreenPtr screen)
+{
+glamor_screen_private *glamor_priv = 
glamor_get_screen_private(screen);

+
+glamor_get_context(glamor_priv);
+
+glGenBuffers(1, glamor_priv-vbo);
+
+

Re: glamor VBO series

[Markus Wick]

There are some trival issues in patch 7 and 9. After fixing them, 
patches 2-9 will be:


Reviewed-by: Markus Wick markus at selfnet.de

Am 2014-03-09 05:07, schrieb Eric Anholt:

Here's a series for the reusable VBO support code in glamor.  The
Render code using it still pretty scary, but it seems to work, and
we're going to want to rewrite all the vertex setup anyway.

After this, we can start getting into the real performance wins from
the new glamor development.

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

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

Re: [PATCH 9/9] glamor: Use buffer_storage

[Markus Wick]

Am 2014-03-10 21:09, schrieb Eric Anholt:

On release builds, glGetError won't be called and so this won't clear
the gl error log. So we'll fall back to the mbr code because of any gl
error somewhere in glamor.


Yeah, I think that's fine -- you shouldn't have any errors, anyway.


But then, we should explicit delete  alloc this buffer again. Atm we 
would fall back to mbr but buffer_storage was maybe successful. So non 
of the next glBufferData will success ...

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

Re: [PATCH 4/6] dix: Remove incorrect comment about privates.

[Markus Wick]

Am 2014-03-06 18:00, schrieb Eric Anholt:

PRIVATE_ALL was apparently dropped before
There wasn't any PRIVATE_ALL key neither in xserver nor in glamor. I 
guess it's just a wrong reference to privates.h, so it's the same as the 
enum DevPrivateKey itself.

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

Re: [PATCH 3/6] glamor: Add a note about the state of GL_ARB_map_buffer_range.

[Markus Wick]

Am 2014-03-06 18:00, schrieb Eric Anholt:

+/* We'd like to require GL_ARB_map_buffer_range or

OpenGL ES 3 or

+ * GL_OES_map_buffer_range, since it offers more information to
+ * the driver than plain old glMapBuffer() or glBufferSubData().
+ * It's been supported on Mesa on the desktop since 2009 and on
+ * GLES2 since October 2012.  It's supported on Apple's iOS
+ * drivers for SGX535 and A7, but apparently not on most Android
+ * devices (the OES extension spec wasn't released until June
+ * 2012).
+ *
+ * 82% of 0 A.D. players (desktop GL) submitting hardware reports
+ * have support for it, with most of the ones lacking it being on
+ * Windows with Intel 4-series (G45) graphics or older.
+ */
 if (glamor_priv-gl_flavor == GLAMOR_GL_DESKTOP) {
 if (gl_version  GLAMOR_GL_VERSION_ENCODE(1, 3)) {
 ErrorF(Require OpenGL version 1.3 or later.\n);


glMapBuffer was dropped in OpenGL ES3, so we also have to check for this 
version to enable glMapBufferRange.
But keep in mind that most current mobile drivers doesn't support unsync 
mapping.

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

Re: little glamor cleanups (and a dix cleanup)

[Markus Wick]

Patches 1-5 are:

Reviewed-by: Markus Wick markus at selfnet.de

Am 2014-03-06 18:00, schrieb Eric Anholt:

Here are some trivial patches from the glamor-server branch for
review.  I'm planning to send out the VBO changes next, which get us
closer to making glamor not actually suck for performance.

These are on the glamor-cleanups branch of my tree, branched from
glamor-pull-request.

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