Re: [PATCH v3] winex11.drv: Make the GL context current even if no drawables were given.
2016-02-24 5:12 GMT+01:00 Charles Davis <cdavis5x(a)gmail.com>:
Signed-off-by: Charles Davis <cdavis5x(a)gmail.com> --- Try 3: Fix an assignment I missed. (Thanks, Ken.)
dlls/opengl32/tests/opengl.c | 2 +- dlls/winex11.drv/opengl.c | 69 +++++++++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c index e5e1507..c058c19 100644 --- a/dlls/opengl32/tests/opengl.c +++ b/dlls/opengl32/tests/opengl.c @@ -975,7 +975,7 @@ static void test_opengl3(HDC hdc) * expect drivers to ever offer it. */ res = wglMakeCurrent(0, gl3Ctx); - ok(res == FALSE, "Wow, OpenGL 3.0 windowless rendering passed while it was expected not to!\n"); + ok(res || broken(!res), "OpenGL 3.0 windowless rendering failed!\n");
That seems suspicious... AFAICS the comment right above this hunk is still valid (it is for current Nvidia drivers on Win7 at least), which probably means that this patch is not a good idea.
On Thu, Feb 25, 2016 at 7:29 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
2016-02-24 5:12 GMT+01:00 Charles Davis <cdavis5x(a)gmail.com>:
Signed-off-by: Charles Davis <cdavis5x(a)gmail.com> --- Try 3: Fix an assignment I missed. (Thanks, Ken.)
dlls/opengl32/tests/opengl.c | 2 +- dlls/winex11.drv/opengl.c | 69
+++++++++++++++++++++++++-------------------
2 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c index e5e1507..c058c19 100644 --- a/dlls/opengl32/tests/opengl.c +++ b/dlls/opengl32/tests/opengl.c @@ -975,7 +975,7 @@ static void test_opengl3(HDC hdc) * expect drivers to ever offer it. */ res = wglMakeCurrent(0, gl3Ctx); - ok(res == FALSE, "Wow, OpenGL 3.0 windowless rendering passed while it was expected not to!\n"); + ok(res || broken(!res), "OpenGL 3.0 windowless rendering failed!\n");
That seems suspicious... AFAICS the comment right above this hunk is still valid (it is for current Nvidia drivers on Win7 at least), which probably means that this patch is not a good idea.
Yeah, I was afraid of that. Truth be told, I don't need this for an application. Besides the fact that OpenGL 3.0+ is supposed to allow this, I added it because I thought it might be useful for wined3d (e.g. being able to create devices without an implicit swapchain, though now that I've looked into that possibility, making that change would be much harder than I thought). In any case, I 'd imagine from your comment that you disagree. If you do, I 'll retract this patch and revert the patch adding this to winemac.drv , since no known apps depend on this . Chip
2016-02-26 10:42 GMT+01:00 Charles Davis <cdavis5x(a)gmail.com>:
On Thu, Feb 25, 2016 at 7:29 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
2016-02-24 5:12 GMT+01:00 Charles Davis <cdavis5x(a)gmail.com>:
Signed-off-by: Charles Davis <cdavis5x(a)gmail.com> --- Try 3: Fix an assignment I missed. (Thanks, Ken.)
dlls/opengl32/tests/opengl.c | 2 +- dlls/winex11.drv/opengl.c | 69 +++++++++++++++++++++++++------------------- 2 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c index e5e1507..c058c19 100644 --- a/dlls/opengl32/tests/opengl.c +++ b/dlls/opengl32/tests/opengl.c @@ -975,7 +975,7 @@ static void test_opengl3(HDC hdc) * expect drivers to ever offer it. */ res = wglMakeCurrent(0, gl3Ctx); - ok(res == FALSE, "Wow, OpenGL 3.0 windowless rendering passed while it was expected not to!\n"); + ok(res || broken(!res), "OpenGL 3.0 windowless rendering failed!\n");
That seems suspicious... AFAICS the comment right above this hunk is still valid (it is for current Nvidia drivers on Win7 at least), which probably means that this patch is not a good idea.
Yeah, I was afraid of that.
Truth be told, I don't need this for an application. Besides the fact that OpenGL 3.0+ is supposed to allow this, I added it because I thought it might be useful for wined3d (e.g. being able to create devices without an implicit swapchain, though now that I've looked into that possibility, making that change would be much harder than I thought). In any case, I'd imagine from your comment that you disagree. If you do, I'll retract this patch and revert the patch adding this to winemac.drv, since no known apps depend on this.
Chip
Yes, wined3d currently depends on the implicit swapchain in a lot of places and needs quite a bit of general cleanup for that first. Activating the GL context without a drawable isn't going to work on Windows and while wined3d on Windows isn't the most important target we try at least not to break it. It shouldn't be necessary either, we could e.g. create a dummy pbuffer in wined3d and make the GL context current on it. About the related winemac.drv patch, I guess it would be better to revert it, yes. Sorry that I didn't realize about its implications earlier. We could in principle change the test with some kind of "todo_wine_if" shenanigans instead but that doesn't seem preferable.
On Feb 26, 2016, at 3:48 PM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
2016-02-26 10:42 GMT+01:00 Charles Davis <cdavis5x(a)gmail.com>:
On Thu, Feb 25, 2016 at 7:29 AM, Matteo Bruni <matteo.mystral(a)gmail.com> wrote:
That seems suspicious... AFAICS the comment right above this hunk is still valid (it is for current Nvidia drivers on Win7 at least), which probably means that this patch is not a good idea.
Yeah, I was afraid of that.
Truth be told, I don't need this for an application. Besides the fact that OpenGL 3.0+ is supposed to allow this, I added it because I thought it might be useful for wined3d […]. In any case, I'd imagine from your comment that you disagree. If you do, I'll retract this patch and revert the patch adding this to winemac.drv, since no known apps depend on this.
[…]
About the related winemac.drv patch, I guess it would be better to revert it, yes. Sorry that I didn't realize about its implications earlier. We could in principle change the test with some kind of "todo_wine_if" shenanigans instead but that doesn't seem preferable.
So the reasons to revert are: a) we don't have a real test case, and b) leaving it in causes a success-in-todo_wine test failure specific to the Mac? Seem somewhat tenuous reasons to revert a change that implements functionality from the spec. *shrugs* I guess I don't feel strongly either way. Chip, it's up to you. -Ken
On 29 February 2016 at 04:33, Ken Thomases <ken(a)codeweavers.com> wrote:
So the reasons to revert are: a) we don't have a real test case, and b) leaving it in causes a success-in-todo_wine test failure specific to the Mac? Seem somewhat tenuous reasons to revert a change that implements functionality from the spec.
Those don't help, but the more important reason is that as far as we know this functionality isn't supported on any Windows OpenGL driver, supposedly because of architectural reasons.
2016-02-29 12:45 GMT+01:00 Henri Verbeet <hverbeet(a)gmail.com>:
On 29 February 2016 at 04:33, Ken Thomases <ken(a)codeweavers.com> wrote:
So the reasons to revert are: a) we don't have a real test case, and b) leaving it in causes a success-in-todo_wine test failure specific to the Mac? Seem somewhat tenuous reasons to revert a change that implements functionality from the spec.
Those don't help, but the more important reason is that as far as we know this functionality isn't supported on any Windows OpenGL driver, supposedly because of architectural reasons.
Yeah. FWIW I checked on AMD Win10 and that's still unsupported.
participants (4)
-
Charles Davis -
Henri Verbeet -
Ken Thomases -
Matteo Bruni