[PATCH 0/2] MR5338: wined3d: Require EXT_framebuffer_object.

[Zebediah Figura (＠zfigura)]

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.

[PATCH 1/2] wined3d: Require EXT_framebuffer_object.

[Zebediah Figura]

From: Zebediah Figura

---
 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);

[PATCH 2/2] wined3d: Remove the OffscreenRenderingMode setting.

[Zebediah Figura]

From: Zebediah Figura

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;

[PATCH 2/2] wined3d: Remove the OffscreenRenderingMode setting.

[Marvin]

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