Re: [PATCH] glamor: swizzle RED to 0 for alpha textures.
Dave Airlie writes: > From: Dave Airlie > > I'm pretty sure Eric suspected this could cause a problem, and > we couldn't find a test. Well loading feedly in firefox seems > to trigger badness that this solves. And, loading a page from gfycat is broken by this change. firefox https://gfycat.com/HoarseCheapAmericankestrel So, we can either have feedly with black text or gfycat with cat videos, but not both at the same time. I'm not sure I can live in a world without both of these. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Disable logic ops when doing compositing
Keith Packard writes: > Eric Anholt writes: > >> Keith Packard writes: >> >>> If the logic op gets left enabled, it overrides the blending >>> operation, causing incorrect contents on the display. >>> >>> Signed-off-by: Keith Packard >> >> Reviewed-by: Eric Anholt > > And it's actually wrong (sigh). Need to do this in all paths through > this function (even the PictOpSrc case), and we only want to do it on > non-ES2 flavors. Here's an updated version. > > From 1ac2ef66369f36ef132f643969ad176db39babe7 Mon Sep 17 00:00:00 2001 > From: Keith Packard > Date: Fri, 13 May 2016 04:25:43 -0700 > Subject: [PATCH xserver] glamor: Disable logic ops when doing compositing [v2] > > If the logic op gets left enabled, it overrides the blending > operation, causing incorrect contents on the display. > > v2: Disable only on non-ES2, but disable even for PictOpSrc Oh, yeah. Even better. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Disable logic ops when doing compositing
Markus Wick writes: > Hi, > > AFAIK we disable logic ops after each usage, so this seems a bit > redundant to me. Did we miss one usages? Nope, GL_COLOR_LOGIC_OP isn't managed like other glEnable/glDisable pairs; it's left at the previous value. Someday we should figure out how we want this to work and make it all consistent. I'd suggest defining the set of glEnable values that are 'global' and having every user set them to the desired value. That way, they will get changed only when actually changing modes, and not just to reset back to a 'base' state, whatever that means, under an assumption that gl will be able to easily optimize calls which set the state to the current value, but that flipping the state between two values may actually cost something. Right now, in glamor, we use: GL_BLEND, GL_COLOR_LOGIC_OP, GL_SCISSOR_TEST > As logic ops aren't very common, this sounds good to me. It's all inconsistent, which is the worst possible plan. -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Disable logic ops when doing compositing
Eric Anholt writes:
> Keith Packard writes:
>
>> If the logic op gets left enabled, it overrides the blending
>> operation, causing incorrect contents on the display.
>>
>> Signed-off-by: Keith Packard
>
> Reviewed-by: Eric Anholt
And it's actually wrong (sigh). Need to do this in all paths through
this function (even the PictOpSrc case), and we only want to do it on
non-ES2 flavors. Here's an updated version.
From 1ac2ef66369f36ef132f643969ad176db39babe7 Mon Sep 17 00:00:00 2001
From: Keith Packard
Date: Fri, 13 May 2016 04:25:43 -0700
Subject: [PATCH xserver] glamor: Disable logic ops when doing compositing [v2]
If the logic op gets left enabled, it overrides the blending
operation, causing incorrect contents on the display.
v2: Disable only on non-ES2, but disable even for PictOpSrc
Signed-off-by: Keith Packard
---
glamor/glamor_program.c | 4
1 file changed, 4 insertions(+)
diff --git a/glamor/glamor_program.c b/glamor/glamor_program.c
index 0a94de6..322d198 100644
--- a/glamor/glamor_program.c
+++ b/glamor/glamor_program.c
@@ -445,6 +445,7 @@ static struct blendinfo composite_op_info[] = {
static void
glamor_set_blend(CARD8 op, glamor_program_alpha alpha, PicturePtr dst)
{
+glamor_screen_private *glamor_priv = glamor_get_screen_private(dst->pDrawable->pScreen);
GLenum src_blend, dst_blend;
struct blendinfo *op_info;
@@ -459,6 +460,9 @@ glamor_set_blend(CARD8 op, glamor_program_alpha alpha, PicturePtr dst)
break;
}
+if (glamor_priv->gl_flavor != GLAMOR_GL_ES2)
+glDisable(GL_COLOR_LOGIC_OP);
+
if (op == PictOpSrc)
return;
--
2.8.0.rc3
--
-keith
signature.asc
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Disable logic ops when doing compositing
Keith Packard writes: > If the logic op gets left enabled, it overrides the blending > operation, causing incorrect contents on the display. > > Signed-off-by: Keith Packard Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] xrandrprovider: Do not use separate lists for unbound / source / offload slaves
A single provider can be both a offload and source slave at the same time,
the use of seperate lists breaks in this case e.g. :
xrandr --listproviders
Providers: number : 2
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink
Offload crtcs: 3 outputs: 2 associated providers: 0 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink
Offload crtcs: 2 outputs: 5 associated providers: 0 name:modesetting
xrandr --setprovideroutputsource 1 0x7b
xrandr --listproviders
Providers: number : 2
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink
Offload crtcs: 3 outputs: 2 associated providers: 1 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink
Offload crtcs: 2 outputs: 5 associated providers: 1 name:modesetting
xrandr --setprovideroffloadsink 1 0x7b
xrandr --listproviders
Providers: number : 3
Provider 0: id: 0x7b cap: 0xf, Source Output, Sink Output, Source Offload, Sink
Offload crtcs: 3 outputs: 2 associated providers: 2 name:modesetting
Provider 1: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink
Offload crtcs: 2 outputs: 5 associated providers: 2 name:modesetting
Provider 2: id: 0x46 cap: 0xf, Source Output, Sink Output, Source Offload, Sink
Offload crtcs: 2 outputs: 5 associated providers: 2 name:modesetting
Not good. The problem is that the provider with id 0x46 now is on both
the output_slave_list and the offload_slave_list of the master screen.
This commit fixes this by unifying all 3 lists into a single slaves list.
Note that this does change the struct _Screen definition, so this is an ABI
break. I do not expect any of the drivers to actually use the removed / changed
fields so a recompile should suffice.
Signed-off-by: Hans de Goede
---
dix/dispatch.c | 30 +++
hw/xfree86/dri2/dri2.c | 7 +--
hw/xfree86/modes/xf86RandR12.c | 7 ++-
include/scrnintstr.h | 15 +++---
present/present.c | 2 +-
randr/randr.c | 42 ++---
randr/rrcrtc.c | 16 +++---
randr/rrmonitor.c | 17 ---
randr/rroutput.c | 10 -
randr/rrprovider.c | 47 +-
randr/rrscreen.c | 12 ---
11 files changed, 102 insertions(+), 103 deletions(-)
diff --git a/dix/dispatch.c b/dix/dispatch.c
index 26122c1..6b893b4 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3790,9 +3790,7 @@ static int init_screen(ScreenPtr pScreen, int i, Bool gpu)
pScreen->CreateScreenResources = 0;
xorg_list_init(&pScreen->pixmap_dirty_list);
-xorg_list_init(&pScreen->unattached_list);
-xorg_list_init(&pScreen->output_slave_list);
-xorg_list_init(&pScreen->offload_slave_list);
+xorg_list_init(&pScreen->slave_list);
/*
* This loop gets run once for every Screen that gets added,
@@ -3951,7 +3949,7 @@ AttachUnboundGPU(ScreenPtr pScreen, ScreenPtr new)
{
assert(new->isGPU);
assert(!new->current_master);
-xorg_list_add(&new->unattached_head, &pScreen->unattached_list);
+xorg_list_add(&new->slave_head, &pScreen->slave_list);
new->current_master = pScreen;
}
@@ -3959,7 +3957,9 @@ void
DetachUnboundGPU(ScreenPtr slave)
{
assert(slave->isGPU);
-xorg_list_del(&slave->unattached_head);
+assert(!slave->is_output_slave);
+assert(!slave->is_offload_slave);
+xorg_list_del(&slave->slave_head);
slave->current_master = NULL;
}
@@ -3967,31 +3967,35 @@ void
AttachOutputGPU(ScreenPtr pScreen, ScreenPtr new)
{
assert(new->isGPU);
-xorg_list_add(&new->output_head, &pScreen->output_slave_list);
-new->current_master = pScreen;
+assert(!new->is_output_slave);
+assert(new->current_master == pScreen);
+new->is_output_slave = TRUE;
+new->current_master->output_slaves++;
}
void
DetachOutputGPU(ScreenPtr slave)
{
assert(slave->isGPU);
-xorg_list_del(&slave->output_head);
-slave->current_master = NULL;
+assert(slave->is_output_slave);
+slave->current_master->output_slaves--;
+slave->is_output_slave = FALSE;
}
void
AttachOffloadGPU(ScreenPtr pScreen, ScreenPtr new)
{
assert(new->isGPU);
-xorg_list_add(&new->offload_head, &pScreen->offload_slave_list);
-new->current_master = pScreen;
+assert(!new->is_offload_slave);
+assert(new->current_master == pScreen);
+new->is_offload_slave = TRUE;
}
void
DetachOffloadGPU(ScreenPtr slave)
{
assert(slave->isGPU);
-xorg_list_del(&slave->offload_head);
-slave->current_master = NULL;
+assert(slave->is_offload_slave);
+slave->is_offload_slave = FALSE;
}
diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
index f80599f..2165603 100644
--- a/hw/xfree86/dri2/dri2.c
+++ b/hw/xfree86/dri2/dri2.c
Re: [PATCH xserver] glamor: Disable logic ops when doing compositing
On Fri, May 13, 2016 at 7:29 AM, Keith Packard wrote: > If the logic op gets left enabled, it overrides the blending > operation, causing incorrect contents on the display. > > Signed-off-by: Keith Packard Makes sense. Reviewed-by: Alex Deucher > --- > 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); > } > > -- > 2.8.0.rc3 > > ___ > 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 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Disable logic ops when doing compositing
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
[PATCH xserver] glamor: Disable logic ops when doing compositing
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); } -- 2.8.0.rc3 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH RESEND xserver v3] xfree86: Immediately handle failure to set HW cursor
On 05/13/2016 01:04 AM, Adam Jackson wrote: On Thu, 2016-05-12 at 16:04 +0900, Alexandre Courbot wrote: There is currently no reliable way to report failure to set a HW cursor. Still such failures can happen if e.g. the MODE_CURSOR DRM ioctl fails (which currently happens at least with modesetting on Tegra for format incompatibility reasons). As failures are currently handled by setting the HW cursor size to (0,0), the fallback to SW cursor will not happen until the next time the cursor changes and xf86CursorSetCursor() is called again. In the meantime, the cursor will be invisible to the user. This patch addresses that by adding _xf86CrtcFuncs::set_cursor_check and _xf86CursorInfoRec::ShowCursorCheck hook variants that return booleans. This allows to propagate errors up to xf86CursorSetCursor(), which can then fall back to using the SW cursor immediately. Signed-off-by: Alexandre Courbot Cc: Michael Thayer --- Hoping to see this patch go through - all issues in the previous versions have been addressed, and the logic it introduces seems saner than setting the cursor size to (0, 0). How are external drivers to know whether .set_cursor_check is available at compile time? ABI_VIDEODRV_VERSION > 21.0 ? (That's a fine answer, just want to make sure it's explicit.) Yes, that's probably how it should be done - should I add a comment somewhere in the code to specify the minimum ABI version? I haven't found any such comment for other _check variants though. ___ 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
