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

GL game "Beyond All Reason" window overestimated by glretrace #874

Open
zmike opened this issue Jun 21, 2023 · 11 comments
Open

GL game "Beyond All Reason" window overestimated by glretrace #874

zmike opened this issue Jun 21, 2023 · 11 comments

Comments

@zmike
Copy link
Contributor

zmike commented Jun 21, 2023

I'm not entirely sure what's going on here, but I recently was debugging this game and couldn't get apitrace to work.

Some instructions in the issue comments here.

Basically if I inject the apitrace wrapper, it records a trace, and I can play it back without errors, but the window doesn't render anything.

@jrfonseca
Copy link
Member

Could you please provide me:

  • list of any apitrace warnings when tracing, as there might be a smoking gun;
  • the trace, as I usually can infer what's going on.

@zmike
Copy link
Contributor Author

zmike commented Jun 23, 2023

@jrfonseca
Copy link
Member

Thanks. For me, the window is not completely blank:

0001255141

So it looks like the window size was overestimated, in height, somehow.

@zmike
Copy link
Contributor Author

zmike commented Jun 23, 2023

🤦

My monitor isn't big enough to see.

@jrfonseca
Copy link
Member

jrfonseca commented Jun 23, 2023

I see what's going on.

The application is using a 691 x 2289 FBO, but it's setting the viewport before it binds the FBO:

...
6053 glBindFramebufferEXT(target = GL_FRAMEBUFFER, framebuffer = 0)
36054 glPushAttrib(mask = GL_VIEWPORT_BIT)
36055 glViewport(x = 0, y = 0, width = 691, height = 2289)
36056 glMatrixMode(mode = GL_PROJECTION)
36057 glPushMatrix()
36058 glLoadIdentity()
36059 glMatrixMode(mode = GL_MODELVIEW)
36060 glPushMatrix()
36061 glLoadIdentity()
36062 glGetIntegerv(pname = GL_DRAW_FRAMEBUFFER_BINDING, params = &0)
36063 glBindFramebufferEXT(target = GL_FRAMEBUFFER, framebuffer = 4)
36064 glClearColor(red = 0, green = 0, blue = 0, alpha = 1)
...

From what I've gathered this is a valid usage, ie, viewport is a global state, not FBO state. However it confuses glretrace into thinking that the window should be larger.

In short, it's a glretrace bug, annoying yet mostly harmless, and it's unclear what's the most effective way to fix it...

@jrfonseca jrfonseca changed the title GL game "Beyond All Reason" doesn't work with apitrace GL game "Beyond All Reason" window overestimated by glretrace Jun 23, 2023
@jrfonseca jrfonseca removed the Tracing label Jun 23, 2023
@okias
Copy link
Contributor

okias commented Aug 13, 2023

Do you have idea about any hacky approach to create reasonable trace? The BAR is pretty complex game and would be great for the Mesa3D CI testing.

@zmike
Copy link
Contributor Author

zmike commented Oct 17, 2023

@jrfonseca any ideas on a resolution here?

@jrfonseca
Copy link
Member

Sorry, there's no easy solution from Apitrace's side.

The fundamental issue is that window size is not explicit in GLX/WGL APIs, instead size is only explicit on X11/GDI APIs which Apitrace does not intercept. So this means that if we want to explicitly record the window size we'd need Apitrace to:

  1. extract the window size while tracing (easy, there is/was code do to this)
  2. emit a fake made up call (e.g, glXSetWindowSizeApitrace()/wglSetWindowSizeApitrace()) first time drawable is bound, or whenever the drawable size changes
  3. handle those fake calls in glretrace to resize the window acordingly, instead of infering size from glViewport.

I'd happily accept patches along these lines.

That said, if all you want is to add BAR to a CI database, then my suggestion is to ignore this issue, and record the reference screenshots as they are (with blank portion, as seen in #874 (comment)) -- it should still work just fine.

If the blank area is annoying when eyeballing rendering, then we could add an option to glretrace to clamp/restrict maximum window size.

@zmike
Copy link
Contributor Author

zmike commented Oct 17, 2023

This ticket and my interest here are both unrelated to CI.

If you want to give me some breadcrumbs towards your proposed solution I don't mind giving it a shot. I've recently tried getting a new trace from this game, but somehow replay is crashing in the first glMultiDrawElementsIndirect when accessing the indirect buffer, so I'll probably be sidetracked onto figuring that out for a bit.

@jrfonseca
Copy link
Member

but somehow replay is crashing in the first glMultiDrawElementsIndirect when accessing the indirect buffer

It's probably better to track this in a separate issue report, but looking at that function, I suspect there's work to do to handle the indirect parameter correctly, namely it's an offset disguised as a pointer when GL_DRAW_INDIRECT_BUFFER is bound, but it's a user memory pointer when not. In short, we need to handle indirect parameter in a similar fashion to indices. See GLindexBuffer in

apitrace/specs/gltypes.py

Lines 305 to 315 in 223c85d

def GLindexBuffer(countExpr, typeExpr):
# Indices arguments are polymorphic:
# - offsets when element array buffer is bound
# - or a blob otherwise.
sizeExpr = '%s*_gl_type_size(%s)' % (countExpr, typeExpr)
return Polymorphic('_element_array_buffer_binding()', [
('0', Blob(Const(GLvoid), sizeExpr)),
],
IntPointer("const GLvoid *"),
contextLess=False,
)

Not saying this is definitely what's causing the issue, but it's likely.

@zmike
Copy link
Contributor Author

zmike commented Oct 17, 2023

I wasn't intending to raise it as an apitrace issue (yet), just saying why I may not get to the issue in this ticket immediately.

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

3 participants