There's a little problem with consistency about these things. In various related routines and paths, there were (and still are) both places that check isInDraw and places that don't check it. read_from_framebuffer and IWineD3DSurfaceImpl_BindTexture that can be called from LoadLocation are examples of each kind. Some kind of single scheme would make it easier to follow. Also even NOP ActivateContext(device, lastActiveRenderTarget, CTXUSAGE_RESOURCELOAD) create some false positives when using debug traces to identify potential problem places where ActivateContext is used inside ENTER_GL/LEAVE_GL.
On the other hand, it is pretty tricky. Also Stefan believes adding isInDraw checks everywhere isn't especially nice, but ultimately to prevent all situations of ActivateContext inside ENTER_GL/LEAVE_GL with isInDraw approach this would've to be added in more places. I'm not sure about this myself, perhaps I'll better not hurry with it and some better ideas may come.
Allan Tong wrote:
I added a comment to the bug, but I'm afraid I don't really know the details about how to reproduce the problem as I never experienced it. I think Stefan was the one who mentioned the rendering problems in HL2 Lost Coast, so maybe he can give you more details?
On another note, what are the thoughts about leaving these ActivateContext calls the way they are now? As things currently stand, a lot of these calls are only interested in making sure there is an active gl context by doing ActivateContext(device, device->lastActiveRenderTarget, CTXUSAGE_RESOURCELOAD). This is essentially a NOP if there was a previous call to ActivateContext, so if you have something like:
ActivateContext(device, target, usage); ... ENTER_GL(); ... ActivateContext(device, lastActiveRenderTarget, CTXUSAGE_RESOURCELOAD); ... LEAVE_GL();
the second ActivateContext should be harmless, even if it's not ideal from a design standpoint.
- Allan