Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong scissor clipping when using glClipControl origin GL_UPPER_LEFT #2186

Closed
MichaelSSky opened this issue Nov 13, 2018 · 7 comments
Closed

Comments

@MichaelSSky
Copy link

MichaelSSky commented Nov 13, 2018

Hi!
In my app I set glClipControl origin to be GL_UPPER_LEFT to match D3D screen space coordinated.It effectively flips frame buffer attachment along Y axis .And I see that imgui GL backend uses inverted scissor rectangle ,and in this case the ui gets cut in wrong places.
My imgui version (v1.60 WIP) is a bit old so I made the following change to make it work:

imgui_impl_opengl3.cpp

//if glCLipControl is GL_UPPER_LEFT we must use ClipRect as is
glScissor((int)pcmd->ClipRect.x, (int)(pcmd->ClipRect.y), (int)(pcmd->ClipRect.z), (int)(pcmd->ClipRect.w));
@ocornut
Copy link
Owner

ocornut commented Nov 13, 2018

Hello,

Never heard of glClipControl() until today! (it is a GL 4.5 function)
In theory we should save the current value using some glGet (I don't know if there is a define for it, can't find it), then call glClipControl(GL_LOWER_LEFT) then restore it.

Since this is so esoteric and using 4.5+ functions may be problematic in the binding, could you just call glClipControl(GL_LOWER_LEFT) because calling the draw function? This way you don't have to modify imgui_impl_opengl3.cpp

@MichaelSSky
Copy link
Author

MichaelSSky commented Nov 13, 2018

Here is how I ended it:

```

GLint clip_origin; glGetIntegerv(GL_CLIP_ORIGIN, &clip_origin);

    ...later

    for (int cmd_i = 0; cmd_i < cmd_list->CmdBuffer.Size; cmd_i++)
    {
        const ImDrawCmd* pcmd = &cmd_list->CmdBuffer[cmd_i];
        if (pcmd->UserCallback)
        {
            pcmd->UserCallback(cmd_list, pcmd);
        }
        else
        {
            glBindTexture(GL_TEXTURE_2D, (GLuint)(intptr_t)pcmd->TextureId);

			if (clip_origin == GL_LOWER_LEFT)
			{
				glScissor((int)pcmd->ClipRect.x, (int)(fb_height - pcmd->ClipRect.w), (int)(pcmd->ClipRect.z - pcmd->ClipRect.x), (int)(pcmd->ClipRect.w - pcmd->ClipRect.y));
			}
			else	//if glCLipControl is GL_UPPER_LEFT we must use ClipRect as is
			{
				glScissor((int)pcmd->ClipRect.x, (int)(pcmd->ClipRect.y), (int)(pcmd->ClipRect.z), (int)(pcmd->ClipRect.w));
			}
           
            glDrawElements(GL_TRIANGLES, (GLsizei)pcmd->ElemCount, sizeof(ImDrawIdx) == 2 ? GL_UNSIGNED_SHORT : GL_UNSIGNED_INT, idx_buffer_offset);
        }
        idx_buffer_offset += pcmd->ElemCount;
    }
}

@ocornut
Copy link
Owner

ocornut commented Nov 13, 2018

Can't find it in the docs of khronos.org but apparently there is a GL_CLIP_ORIGIN to be used with glGet()
https://www.khronos.org/registry/OpenGL/extensions/ARB/ARB_clip_control.txt

New Tokens

    Accepted by the <origin> parameter of ClipControl:

        LOWER_LEFT                                  0x8CA1
        UPPER_LEFT                                  0x8CA2

    Accepted by the <depth> parameter of ClipControl:

        NEGATIVE_ONE_TO_ONE                         0x935E
        ZERO_TO_ONE                                 0x935F

    Accepted by the <pname> parameter of GetBooleanv, GetIntegerv,
    GetFloatv, and GetDoublev:

        CLIP_ORIGIN                                 0x935C
        CLIP_DEPTH_MODE                             0x935D

@MichaelSSky
Copy link
Author

See my update. I don't frequently update imgui so I just fixed it inside that file.But maybe for the future uses it should be added via state query as you do with the rest of params.
Thanks.

@ocornut
Copy link
Owner

ocornut commented Nov 13, 2018

My suggest above was to change the clip control on your side so you don't modify imgui's code. This is only going to reduce your update flexibility.

I have now added support for it in imgui_impl_opengl3.cpp (untested).
This requires the GL header file accessed by imgui_impl_opengl3 to be defining GL_CLIP_ORIGIN so may not work properly in all loaders configuration. How does it works for you?

@MichaelSSky
Copy link
Author

MichaelSSky commented Nov 13, 2018

I see what you mean,but in my code I try to hide OpenGL details as much as possible ,and because imgui is used just for debug tasks and the rest of the rendering system uses UPPER_LEFT origin mode,in my case I found it more suitable to put inside that rendering backend file. But if you want my opinion so I would just do it as you do with the rest of the state.Just query that clip state and then either change the state during imgui pass or change the clipping rectangle.I hate changing states too much so I went with the second one .
So your patch looks great.Thanks.

@ocornut ocornut closed this as completed Nov 13, 2018
ocornut added a commit that referenced this issue Nov 21, 2018
… glGetIntegerv(GL_CLIP_ORIGIN is not honored properly. (#2186, #2195) Fix f52f0a5
@ocornut
Copy link
Owner

ocornut commented Feb 19, 2019

Hello @MichaelSSky ,
Do you know if GL_CLIP_ORIGIN / GL_UPPER_LEFT if available in some form on Mac/OSX ?
Are you using it there?
See #2366

ocornut added a commit that referenced this issue Feb 20, 2019
… to read GL_CLIP_ORIGIN even if the OpenGL headers/loader happens to define the value. (#2366, #2186)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants