View Issue Details

IDProjectCategoryView StatusLast Update
0001923NoesisGUIC# SDKpublic2021-02-18 20:31
ReporterDavidYawCSpeedAssigned Tosfernandez 
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.0.10 
Target Version3.0.11Fixed in Version3.0.11 
Summary0001923: Inconsistent behavior in OpenGL Render Contexts
DescriptionI have a cross-platform application that uses the WGL, GLX, or WGL Render Context. In 3.0.10, some changes were made to make the behavior of these render contexts different. The behavior of these OpenGL contexts should be the same.

I'm focusing on method CaptureRenderTarget.

https://github.com/Noesis/Managed/blob/master/Src/NoesisApp/RenderContexts/WGL/Src/RenderContextWGL.cs#L180
https://github.com/Noesis/Managed/blob/master/Src/NoesisApp/RenderContexts/GLX/Src/RenderContextGLX.cs#L191
https://github.com/Noesis/Managed/blob/master/Src/NoesisApp/RenderContexts/EGL/Src/RenderContextEGL.cs#L131

In 3.0.10, WGL got two changes:
- Flip the image to be right-side up: int srcRow = (height - i - 1) * srcStride;
- Don't change the order of bytes in a pixel: // RGBA -> BGRA --> Noesis.Marshal.Copy

The first change is good, we no longer have to render with a Transform containing ScaleY = -1. But it should be made to all render contexts.

The second change is not as good. This ends up flipping the colors from what we expected: An orange original ends up being captured as light blue.

The behavior of the three OpenGL render contexts should be the same. My recommendation would be to add the image flip on GLX and EGL, and to re-add the byte order change on WGL.
TagsNo tags attached.
PlatformAny

Activities

sfernandez

sfernandez

2021-02-15 20:00

manager   ~0007039

Are you sure that color order in WGL context is not correct now?
Because I changed it after trying to implement the 2 displays scenario in Windows using WGL render context. I noticed that updating the texture with the pixels from the ImageCapture was giving the wrong color order.
Are you using the pixel array to update a texture also?
DavidYawCSpeed

DavidYawCSpeed

2021-02-15 20:29

reporter   ~0007040

Last edited: 2021-02-15 21:48

View 3 revisions

I performed the following:

- CaptureRenderTarget.
- Transform the pixels to a Bitmap, with no manipulation of either bytes-within-pixel or row ordering.
   - new System.Drawing.Bitmap(width, height, System.Drawing.Imaging.PixelFormat.Format32bppArgb)
   - bitmap.LockBits
   - For each row, Marshal.Copy(imagecapture.pixels, row * width * pixelSize, bitmap.Scan0 + bitmap.Stride * row, width * pixelSize);
- Save to disk with bitmap.Save("TestSave.jpg")

With the 3.0.10 WGL render context, the image comes out light blue, while the original is tan/yellow/orange. With the 3.0.10 EGL render context, it comes out the proper tan/yellow/orange.

DavidYawCSpeed

DavidYawCSpeed

2021-02-16 20:20

reporter   ~0007042

To be precise: Whether the render context re-orders the colors within the pixel, or whether it flips the image vertically, it doesn't matter as long as the various OpenGL contexts do the same thing, so that it behaves the same on multiple platforms. My preferences for what they do are listed above, but as long as they all do the same thing, we can make it work in our application.
sfernandez

sfernandez

2021-02-16 20:41

manager   ~0007043

My motivation to change the pixel color order was to improve the 2-window scenario where a render target was captured into a byte buffer and then used to update a texture for the secondary window. Without that change I needed to do an extra copy of the buffer to reorder the pixels for the texture update.
Anyway, after the poor framerate results in your device that path is totally discarded, any approach that implies reading the pixels in the CPU and updating a texture is not going to work.

We think that CaptureRenderTarget should return the pixels in a format more useful for writing images to disk, that would be BGRA as we had in previous versions.

I will apply the Y inversion to all OpenGL render contexts and unify the pixel color ordering for the next version as indicated.
DavidYawCSpeed

DavidYawCSpeed

2021-02-17 01:11

reporter   ~0007044

Last edited: 2021-02-17 01:21

View 3 revisions

Agreed, using the same format as the second window texture would be good for performance, but the performance isn't there on our hardware, so that concern is moot.

Perhaps at some point in the future, CaptureRenderTarget could have parameters specifying the desired pixel format and row order.

I did some additional testing:

- In the D3D render: I tried specifying Format.B8G8R8A8_UNorm[_SRgb] instead of the existing RGB in the Init() method. Unfortunately, this did not result in any change, in either the primary window, secondary window, or the saved JPG.

- In the WGL render: I tried specifying GL_BGRA = 0x000080E1 in the call to glReadPixels in CaptureRenderTarget. This reversed the RGB/BGR colors. I only tested this on Windows, but this seems like it would be better performance than reversing the colors so manually.

- Also in the WGL render: I notice that "byte[] src = new byte[width * height * 4];" is called for each frame captured. This is spamming the garbage collector, in the same way that creating a new Image Capture object used to. Since the strides are equal, perhaps directly pass _imageCapture.Pixels to glReadPixels, and do an in-place re-ordering of the rows? Or cache the 'src' array in a similar manner to _imageCapture.

Issue History

Date Modified Username Field Change
2021-02-15 19:24 DavidYawCSpeed New Issue
2021-02-15 20:00 sfernandez Assigned To => sfernandez
2021-02-15 20:00 sfernandez Status new => feedback
2021-02-15 20:00 sfernandez Note Added: 0007039
2021-02-15 20:01 sfernandez Target Version => 3.0.11
2021-02-15 20:29 DavidYawCSpeed Note Added: 0007040
2021-02-15 20:29 DavidYawCSpeed Status feedback => assigned
2021-02-15 20:29 DavidYawCSpeed Note Edited: 0007040 View Revisions
2021-02-15 21:48 DavidYawCSpeed Note Edited: 0007040 View Revisions
2021-02-16 20:20 DavidYawCSpeed Note Added: 0007042
2021-02-16 20:41 sfernandez Status assigned => feedback
2021-02-16 20:41 sfernandez Note Added: 0007043
2021-02-17 01:11 DavidYawCSpeed Note Added: 0007044
2021-02-17 01:11 DavidYawCSpeed Status feedback => assigned
2021-02-17 01:12 DavidYawCSpeed Note Edited: 0007044 View Revisions
2021-02-17 01:21 DavidYawCSpeed Note Edited: 0007044 View Revisions
2021-02-18 20:31 sfernandez Status assigned => resolved
2021-02-18 20:31 sfernandez Resolution open => fixed
2021-02-18 20:31 sfernandez Fixed in Version => 3.0.11