Rendering fixes

[smilediver]

This PR contains two fixes:

Backend: remove RenderTargetFlag and refactor depth/stencil state setup

First fix removes flags from RenderTarget. This fixes the problem where depth and stencil flags in RenderTarget were desynchronizing from the Renderer state. And this also fixes the issue where a single GPU render pass unnecessarily was split into several incurring a performance hit, because backends instead of changing depth or stencil state were actually changing render targets.

RenderTarget should only represent a stateless thing being rendered into (a framebuffer or a render texture), so it should be just a collection of color/depth/stencil attachments. For some reason it also contained flags for which of those buffers should be enabled or disabled, but it's duplicating the state already tracked in Renderer. Renderer can check what attachments are available in the RenderTarget and what state is set and provide the combined and exact information for what attachments to enable to the backend.

Fix Z-test and Z-write being enabled by default for main queue

Second fix by default disables Z-write and Z-tests for 2D sprites for the main queue, and enables them for 3D models.

This fixes the issue that Z-write and Z-test was enabled by default for 2D sprites only in the main render queue. It causes issues when mixing 2D sprites and 3D models. This is possibly an oversight, because they are enabled only for the main queue, and for GroupCommand queues (e.g. render textures) they are disabled. So this makes all render queues behave the same.

[halx99]

This PR affect core renderer backend, we need more time to test.

[smilediver]

Sure, there's no rush. On my side I did test on Windows, macOS, Android and iOS.

[halx99]

Do you run all tests of cpp-tests on macOS, because the macOS metal backend will throw exception if any render state incorrect

[halx99]

lgtm, merged

[smilediver]

Do you run all tests of cpp-tests on macOS, because the macOS metal backend will throw exception if any render state incorrect

I didn't run all the cpp-tests, because auto test doesn't work (it never worked for me, and always crashed). But I did run some tests related to graphics, and all seemed to work. As well as tested on our game, which has some complex rendering stuff like mixing 2D/3D, using render textures, etc.

[solan-solan]

I stumbled with issue according to this fix, specifically to changing inside RenderTargetGL.cpp
There is the following case to render shadows in my project

First frame:

1. Render to first depth texture for first shadow map cascade (one Depth attachment, no any Color attachments)
2. Render to second depth texture for second shadow map cascade (one Depth attachment, no any Color attachments)
3. Render to third color/depth texture which gets all scene (uses Color/Depth attachment)

Second frame:

4. Render to first depth texture... -> error occurs on the trying to clear first depth texture

====================
The first screen demonstrate how it worked before. The glFrameBufferTexture2D was called even if no color attachments exists by the reason of _flags was equal 1. To my understand it caused that color attachment was properly switched off if it was not introduced (as in my case for shadow textures).

Now it looks like this

glDrawBuffers is called for null array without glFrameBufferTexture2D. There is no issue if color attachment was not applied before.
But when it was applied (steps 3 -> step 4) I see the strange stacktrace. Debugger shows double step on Line 124

And then:

[halx99]

@smilediver do you have any idea?

[rh101]

I stumbled with issue according to this fix, specifically to changing inside RenderTargetGL.cpp There is the following case to render shadows in my project

Is there an existing test in the cpp-tests project that reproduces this? If not, would it be possible for you to add one, or make available a minimal project that reproduces the issue?

@halx99 If this issue is not easily solvable, then would it make sense to revert these changes, and have them in a different branch until they're tested and working correctly?

[halx99]

I stumbled with issue according to this fix, specifically to changing inside RenderTargetGL.cpp There is the following case to render shadows in my project

Is there an existing test in the cpp-tests project that reproduces this? If not, would it be possible for you to add one, or make available a minimal project that reproduces the issue?

@halx99 If this issue is not easily solvable, then would it make sense to revert these changes, and have them in a different branch until they're tested and working correctly?

agree

[halx99]

reverting

[solan-solan]

I updated https://github.com/solan-solan/HeightMap/commits/smooth_lod_passing to correspond it for [a13c6bf] and [0ea9efd] branches, check it please.

By the way RenderTarget::_flags work inconsistence now.
They set here at first when RenderTarget is created

Then, if I properly understand, they should be changed for each separate CommandBufferGL like here for depth state:

But _currentRT->addFlag/removeFlag works with error for me:

_flags gets 1 everytime after _currentRT->addFlag(TargetBufferFlags::DEPTH); and 33 after _currentRT->removeFlag(TargetBufferFlags::DEPTH) it should be related with this macros

(Can you confirm it?)

In other words RenderTarget::_flags == 1 always for RenderTargetGL::update(). (By side effect?)

Besides its never reset COLOR like it supposed that one color attachment would be anyway.
Really this render changes looks good if you can confirm inconsistence of RenderTarget::_flags.

I use this workaround now:

void RenderTargetGL::update() const {
    if (!_dirty) return;
    if(!_defaultRenderTarget) {
        { // color attachments
            GLenum bufs[MAX_COLOR_ATTCHMENT] = {GL_NONE};
            for (size_t i = 0; i < MAX_COLOR_ATTCHMENT; ++i)
            {
                if (_color[i] || !i)

[halx99]

I updated https://github.com/solan-solan/HeightMap/commits/smooth_lod_passing to correspond it for [a13c6bf] and [0ea9efd] branches, check it please.

By the way RenderTarget::_flags work inconsistence now. They set here at first when RenderTarget is created

Then, if I properly understand, they should be changed for each separate CommandBufferGL like here for depth state:

But _currentRT->addFlag/removeFlag works with error for me:

_flags gets 1 everytime after _currentRT->addFlag(TargetBufferFlags::DEPTH); and 33 after _currentRT->removeFlag(TargetBufferFlags::DEPTH) it should be related with this macros (Can you confirm it?)

In other words RenderTarget::_flags == 1 always for RenderTargetGL::update(). (By side effect?)

Besides its never reset COLOR like it supposed that one color attachment would be anyway. Really this render changes looks good if you can confirm inconsistence of RenderTarget::_flags.

I use this workaround now:

void RenderTargetGL::update() const {
    if (!_dirty) return;
    if(!_defaultRenderTarget) {
        { // color attachments
            GLenum bufs[MAX_COLOR_ATTCHMENT] = {GL_NONE};
            for (size_t i = 0; i < MAX_COLOR_ATTCHMENT; ++i)
            {
                if (_color[i] || !i)

checked, it's correct, due to the TargetBufferFlags is uint8_t, the debugger display char, it's num value is 49

[halx99]

[halx99]

I reset axmol to 42bf253 and build your https://github.com/solan-solan/HeightMap/commits/smooth_lod_passing, and the GL errors occured:

[halx99]

due to _flags was removed, need update color attachment always

[halx99]

@solan-solan remove the condition check _color[i] can solve above GL errors

[solan-solan]

@solan-solan remove the condition check _color[i] can solve above GL errors

@halx99 I believe that it helps

[smilediver]

I'll have a look at this issue next week.

[halx99]

I'v rebase and drop the merge commit, and a backup branch was created: https://github.com/axmolengine/axmol/tree/refactor-render-target , you can fork it and continue working on it, then create a new PR