2016-11-22 5:48 GMT+01:00 Andrew Wesie awesie@gmail.com:
On Mon, Nov 21, 2016 at 6:51 PM, Matteo Bruni matteo.mystral@gmail.com
The width and height values that are in viewport_test (10000 and 10000) seem arbitrary. Does that test reflect a real world program?
Probably arbitrary values, yeah.
I am trying to figure out why that block of code is even necessary. At the moment, it just appears to make viewport_test work, which is only useful if viewport_test reflects a real world program. My best guess was that the code was introduced to solve a problem when context->render_offscreen is false; hence I moved the code rather than eliminate it entirely.
Somehow I spent most of the day looking into this. I extended the d3d9 test a bit and ran them on a couple of boxes here. It seems to me that, bugs with huge values aside, clamping the viewport values is never expected in d3d9. You're probably on point in that the code was there for the !render_offscreen case, specifically to get some sensible result from the (height - (vp.y + vp.height)) part. I still haven't tested other (in particular, older) d3d versions though.
What is Overwatch depending on specifically? If it sets its viewport partially offscreen and doesn't like the clamping, we should probably make that chunk of code be (indirectly) conditional on the d3d version by means of a wined3d flag.
Overwatch renders some UI elements to a texture that is smaller than the viewport. By clamping the viewport, the elements are now drawn incorrectly because you just changed the mapping from clip space to screen space.
I am not convinced that the original code is correct for any d3d version. The behavior I noted above with regards to testing different heights and widths demonstrates that pretty clearly. I can submit a patch to the test to make it clearer, if that is needed.
Yeah, that makes sense, my point was that it's never going to be right to clamp the viewport only in the !render_offscreen case, as you do in this patch: that flag is a wined3d "implementation detail" and not directly related to d3d state, application-visible behavior can't be dependent on it.
WRT affected wined3d code: aside from the viewport clamping and the viewport flipping in the !render_offscreen case, there is some (not critical) code related to depth buffer location / size tracking in wined3d which doesn't quite expect the viewport to be larger than the render target.
I'm going to cleanup my test patch and port the test to the other d3d versions, hopefully I'll have something useful in the next few days.