[PATCH 0/2] MR5338: wined3d: Require EXT_framebuffer_object.
This is the first of a couple of patch sets which intend to impose some new requirements on GL support. In total I intended to submit patches to require these things: * EXT_framebuffer_object * ARB_texture_non_power_of_two (including WINED3D_GL_NORMALIZED_TEXRECT) * GLSL 1.20 And, unless demand to the contrary is demonstrated, to remove the code paths that care about their absence. This means that the "OffscreenRenderingMode" setting, the ARB shader backend, and all non-GLSL fixed-function pipeline backends will be removed. All of these extensions are at least 17 years old at the current date. -- Some explanation may be warranted. In general Wine is reticent to impose heavy requirements on users' software or hardware, and when it comes to Direct3D we have been especially conservative in this respect. Critically, this is not necessarily a matter of being realistically concerned about hardware or software still in use—although we do tend to be more conscious than most projects about some users' long-term support requirements—but more importantly that we do not waste time removing features just because they are believed to be obsolete. Even where related code is being touched, and hence removing code may seem helpful, it is often easy and generally better to abstract away and "quarantine" that code. The ARB shader backend, for example, is very much quarantined in this way; it is not difficult to modify the GLSL backend and leave the ARB backend alone. With that said, as far as the listed extensions are concerned, there are some reasons I would like to impose these requirements anyway. * In the first case, not all code can be easily "quarantined". Support for ORM_BACKBUFFER, in particular, is very invasive, and affects a large number of disparate places in small ways, rather than being a single alternate path. Support for NP2 fixup is similar, although perhaps a bit less invasive than ORM_BACKBUFFER. * Even with code that can be quarantined, there are some smaller, but I believe nonnegligible, costs to keeping that code around. Speaking in concrete terms so as to be more understandable, if I wish to refactor the way states are tracked and invalidated, I need to do some work to either refactor other backends the same way as the GLSL backend (e.g. 5126), or adjust the quarantine so that the old path is kept around just for quarantined code. Doing the latter has a tendency to build up and make the code difficult to follow. A new contributor can easily get led down the wrong path that way. * Somewhat relatedly, when some functionality or internal helper needs to be fixed, a contributor (and the maintainer) needs to then either spend time fixing *every* backend, or know that some backends don't matter and ignore them [if ignoring is even feasible for a given change]. The latter can be difficult to discover, especially for a new contributor, and even then it takes time—and, in a malleable sense, maintenance burden—to look through code and confirm that it doesn't matter. To be clear, this is not meant as rationale to start removing anything and everything. As with all things, these are tradeoffs, between all of the above reasons to keep or remove any given code. The three listed GL extensions are the only three which I believe, at the moment, that there is enough utility in requiring, and little enough reason to keep. This was a matter of some internal discussion among Direct3D contributors. However, there is not really a consensus — or at least not an enthusiastic consensus — that this is desirable. The above is an attempt to summarize the concerns and justify why I personally think this is a good idea. But it's a difficult decision, and that's at least in part because this code is the best translation layer from old Direct3D to *contemporaneous* GL, and it's sad to see that go. At the same time, in the world of version control, no code is ever truly gone. Hence perhaps the most simple argument to make is that if we are really going to *maintain* this code, that will take extra work—work which at this point I really would rather not do—and if we are not going to maintain it, then we're not offering any more utility than an actual git archive. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5338
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/wined3d/adapter_gl.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/dlls/wined3d/adapter_gl.c b/dlls/wined3d/adapter_gl.c index 2d8018f8cc9..815879e356e 100644 --- a/dlls/wined3d/adapter_gl.c +++ b/dlls/wined3d/adapter_gl.c @@ -3783,6 +3783,8 @@ static BOOL wined3d_adapter_init_gl_caps(struct wined3d_adapter_gl *adapter_gl, gl_info->fbo_ops.glBlitFramebuffer = gl_info->gl_ops.ext.p_glBlitFramebuffer; gl_info->fbo_ops.glGenerateMipmap = gl_info->gl_ops.ext.p_glGenerateMipmap; gl_info->fbo_ops.glFramebufferTexture = gl_info->gl_ops.ext.p_glFramebufferTexture; + + gl_info->supported[EXT_FRAMEBUFFER_OBJECT] = TRUE; } else { @@ -5299,6 +5301,16 @@ static BOOL wined3d_adapter_gl_init(struct wined3d_adapter_gl *adapter_gl, LUID primary_luid, *luid = NULL; unsigned int i; + static const struct + { + enum wined3d_gl_extension extension; + const char *string; + } + required_extensions[] = + { + {EXT_FRAMEBUFFER_OBJECT, "EXT_framebuffer_object"}, + }; + TRACE("adapter_gl %p, ordinal %u, wined3d_creation_flags %#x.\n", adapter_gl, ordinal, wined3d_creation_flags); @@ -5357,6 +5369,16 @@ static BOOL wined3d_adapter_gl_init(struct wined3d_adapter_gl *adapter_gl, return FALSE; } + for (unsigned int i = 0; i < ARRAY_SIZE(required_extensions); ++i) + { + if (!gl_info->supported[required_extensions[i].extension]) + { + ERR("Required extension %s is not supported.\n", required_extensions[i].string); + wined3d_caps_gl_ctx_destroy(&caps_gl_ctx); + return FALSE; + } + } + gl_info->filling_convention_offset = wined3d_adapter_find_fill_offset(&caps_gl_ctx); wined3d_adapter_gl_init_d3d_info(adapter_gl, wined3d_creation_flags); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5338
From: Zebediah Figura <zfigura(a)codeweavers.com> FBOs are required now. --- dlls/wined3d/adapter_gl.c | 4 ---- dlls/wined3d/wined3d_main.c | 3 --- 2 files changed, 7 deletions(-) diff --git a/dlls/wined3d/adapter_gl.c b/dlls/wined3d/adapter_gl.c index 815879e356e..46e580b0ce6 100644 --- a/dlls/wined3d/adapter_gl.c +++ b/dlls/wined3d/adapter_gl.c @@ -5400,10 +5400,6 @@ static BOOL wined3d_adapter_gl_init(struct wined3d_adapter_gl *adapter_gl, TRACE("Reporting (fake) driver version 0x%08x-0x%08x.\n", driver_info->version_high, driver_info->version_low); - if (wined3d_settings.offscreen_rendering_mode == ORM_BACKBUFFER) - ERR_(winediag)("You are using the backbuffer for offscreen rendering. " - "This is unsupported, and will be removed in a future version.\n"); - wined3d_adapter_init_fb_cfgs(adapter_gl, caps_gl_ctx.dc); /* We haven't found any suitable formats. This should only happen in * case of GDI software rendering, which is pretty useless anyway. */ diff --git a/dlls/wined3d/wined3d_main.c b/dlls/wined3d/wined3d_main.c index 0dd2c59844e..2bc54299ce1 100644 --- a/dlls/wined3d/wined3d_main.c +++ b/dlls/wined3d/wined3d_main.c @@ -375,9 +375,6 @@ static BOOL wined3d_dll_init(HINSTANCE hInstDLL) ERR_(winediag)("The GLSL shader backend has been disabled. You get to keep all the pieces if it breaks.\n"); TRACE("Use of GL Shading Language disabled.\n"); } - if (!get_config_key(hkey, appkey, env, "OffscreenRenderingMode", buffer, size) - && !strcmp(buffer,"backbuffer")) - wined3d_settings.offscreen_rendering_mode = ORM_BACKBUFFER; if (!get_config_key_dword(hkey, appkey, env, "VideoPciDeviceID", &tmpvalue)) { int pci_device_id = tmpvalue; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5338
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=144132 Your paranoid android. === debian11b (64 bit WoW report) === kernel32: console.c:4937: Test failed: CreateProcess failed: 1359 "C:\users\winetest\Temp\console_gui.exe" console check_console console.c:4940: Test failed: Could not wait for the child process: -1 le=6 console.c:4943: Test failed: Couldn't get exit_code
participants (3)
-
Marvin -
Zebediah Figura -
Zebediah Figura (＠zfigura)