We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 5015

Summary: SDL_RenderReadPixels on DirectX 11.1 backend seems to be broken
Product: SDL Reporter: Konrad <iryont>
Component: renderAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: major    
Priority: P2 Keywords: target-2.0.14
Version: HG 2.1   
Hardware: x86_64   
OS: Windows 10   
Attachments: Fix

Description Konrad 2020-03-07 01:22:48 UTC
Hello

I didn't have time to look at the code yet, but it appears that SDL_RenderReadPixels on DirectX 11.1 is broken with pixels being messed up:

https://i.imgur.com/89gdAL5.png

Any other backend appears to be working fine.
Comment 1 Konrad 2020-03-10 22:57:00 UTC
This issue happens because of ID3D11DeviceContext1_DiscardView within D3D11_RenderPresent.

Microsoft documentation:

Remarks
DiscardView informs the graphics processing unit (GPU) that the existing content in the resource view that pResourceView points to is no longer needed.

By commenting it out our backbuffer remains untouched and we can copy its data (SDL_RenderReadPixels).

The question is - do we really need that call in D3D11_RenderPresent? I cannot think of any particular reason for it to be there.
Comment 2 Sam Lantinga 2020-03-10 23:19:39 UTC
Are you trying to read the backbuffer after a present call? That's not defined behavior, you should always read after rendering, before the present.

To answer your question, that call allows the graphics driver to optimize the present, possibly avoiding a buffer copy.
Comment 3 Konrad 2020-03-11 07:43:46 UTC
You might be right, however, in most common scenarios you do draw first and then later on you do parse events (e.g. from SDL2 - screenshot hotkey). That being the case, it always happens after the present as it might allow for some room on polling events before attempting to draw another frame.

However, then again I can imagine a scenario in which the resize event is parsed just before the screenshot event in which case it would invalidate the backbuffer anyway. That being the case, I can live with that behavior as I can just defer the task of making the screenshot to the next frame and execute it before the present.

I do appreciate it, thank you. I suppose we can close this issue, but I do highly suggest to explicitly mention that within description of SDL_RenderReadPixels.
Comment 4 Konrad 2020-03-11 07:45:38 UTC
Marking this bug report as resolved (invalid).
Comment 5 Konrad 2020-04-05 11:28:03 UTC
I have to re-open this issues with a small adjustment to the previous report (which turned out to be invalid due to discard view after SDL_RenderPresent).

It appears that I cannot use SDL_RenderReadPixels on a bound framebuffer (SDL_Texture set as render target) as it simply results in gibberish data. However, drawing that framebuffer into the default target (window surface) does render it correctly. Other backends (OpenGL, software, Direct3D) do work fine.

It looks to me like D3D11_RenderReadPixels just gets the general backbuffer and not the current render target and its backbuffer.
Comment 6 Konrad 2020-04-05 13:23:38 UTC
Created attachment 4289 [details]
Fix

Here is the patch which actually fetches the current render target and its underlying ID3D11Resource which is ID3D11Texture2D.
Comment 7 Sam Lantinga 2020-04-05 15:59:14 UTC
Patch added, thanks!
https://hg.libsdl.org/SDL/rev/c3055b205671