Wrong scissor clipping when using glClipControl origin GL_UPPER_LEFT #2186

New issue

Closed

opened 2018-11-13 17:27:40 +00:00 by MichaelSSky · 7 comments

MichaelSSky commented

2018-11-13 17:27:40 +00:00

(Migrated from github.com)

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

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 commented

2018-11-13 17:32:13 +00:00

(Migrated from github.com)

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

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 commented

2018-11-13 17:43:39 +00:00

(Migrated from github.com)

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

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 commented

2018-11-13 17:44:12 +00:00

(Migrated from github.com)

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

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 commented

2018-11-13 17:45:54 +00:00

(Migrated from github.com)

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.

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 commented

2018-11-13 17:55:56 +00:00

(Migrated from github.com)

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?

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 commented

2018-11-13 18:06:20 +00:00

(Migrated from github.com)

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.

👍 1

ocornut commented

2019-02-19 22:21:26 +00:00

(Migrated from github.com)

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

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