Page MenuHomePhabricator

OpenGL rendering broken when VTK renderwindow has viewport
Closed, ResolvedPublic

Description

I noticed this, while trying to use only part of a renderwindow for actual rendeing. There are multiple use cases for this with the most simple being that a certain combination of operating system, OpenGL implementation and remote desktop system returns a renderwindow size like (240,128) when you ask for (128,128).

While working on a fix for this behavior I had to learn the coordinate systems used in GL rendering (in MITK) from code, so I want to add some documentation pieces on that topic --> More detailed description to be found in the wiki page (linked in the upper right corner), meant to be copied into Doxygen documentation.

After we agree on a proper fix, I'd like to add a proper rendering test to make sure this behavior keeps working.

Event Timeline

By now I created a test for rendering into a VTK viewport. To execute the test, additional data is needed in the clone of MITK-Data. This data is attached as an archive. (I don't want to push it before getting positive feedback)

Branch bug-13543-viewport-rendering contains a proposed fix for mitkVtkPropRenderer.cpp and a regression test as part of Module PlanarFigure. Comments are very welcome

Executed manually, the test can be used to verify the fix and to see what has been changed. Unfortunately, the test is ending with an exception in the VTK regression testing class. I'll try to talk to one of the rendering test guys tomorrow.

New remote branch pushed: bug-13543-viewport-rendering

Difference screen shot of the test to ground truth

mitkViewportRenderingTest_ImageSurfacePlanarFigure.diff.png (500×500 px, 2 KB)

Regarding the test:
The test works fine for me, but you should remove the changes to the argv variable, to keep -T option of the VTK test macro working. However, the rendering of the planar figures does not work correctly in the test. They don't have any color. See my attached screen shot.

I pushed some simple changes into a personal branch to make the -T option of VTK work:

personal/kilgus/bug-13543-AnpassungenFuerVTKOption

New remote branch pushed: bug-13543-viewport-rendering-wo-spaces

I guess this is late for integration into the release and I wouldn't mind much if it doesn't get in, but since the target milestone is still 2012-12, I'm requesting the flag. Would be a nice bugfix to the release.

After Thomas' comments and Markus/Erics explanations (private correspondence) I made a minor change to the viewport/scale code in mitkVtkPropRender, which still keeps the output correct, which is verified by the unit test now.

As a side effect of this ticket, I moved Thomas' mitkRenderingTestHelper into the Mitk module, which makes the class accessible to other module tests, which want to verify some mapper behavior by unit tests.

The cleaned up code, free of misleading commits and excessive whitespace is now in branch bug-13543-viewport-rendering-wo-spaces

http://mitk.org/git/?p=MITK.git;a=shortlog;h=refs/heads/bug-13543-viewport-rendering-wo-spaces

Reseting release flag. We decided not to include this fix for the upcoming release

Update: I just discovered that my test case was a little too optimistic. It worked well with quadratic windows and viewports.

With non-quadratic viewports or even more with non-quadratic images, we only get the scale of objects right. The position of objects on the X axis is still broken. I need to investigate how this can be corrected. Also I will expand the one test to more cases with non-quadratic viewports and non-quadratic images.

New remote branch pushed: bug-13543-viewport-tested-snapshot-based

New remote branch pushed: bug-13543-viewport-tested

Another update: yesterday I implemented a modification to Enable2DOpenGL(), which makes rendering right both in viewports and full windows, with quadratic or non-quadratic images and quadratic or other render windows.

I implemented a number of rendering tests, which do test the rendering behavior of PlanarFigue and Surface objects in conjunction with Image objects. Tests vary width and height of the different parameters. Two of the tests do NOT run successfully, but this is a problem in the rendering testframework. When I stop execution and look at the result, everything is right. It just becomes wrong the instance vtkRegressionTestImage is called. So I would keep these two tests for later and close this issue here.

Also I want to ask the OpenGL gurus at DKFZ to verify my fix and document more clearly how and why OpenGL rendering in MITK is supposed to work.

This bug could not be fixed for release 2013-06. Setting target milestone to next release

New remote branch pushed: bug-13543-viewport-last-final

(in our previous discussion of this issue I had a wrong idea of glOrtho() and Markus never though this was an actual issue)

Last tuesday we had a meeting where I could discuss this in detail with Markus Fangerau. This led to better understanding on both parts and allowed me to finalize this branch, including a complete set of tests.

In essence, the goal of this bug is to make VTK and OpenGL based renderers behave the same when the vtkRenderer is given a viewport. This is achieved now.

Markus brought up the valid point that rendering into a viewport will probably lead to interaction problems (measuring pixel distances, running them through DisplayGeometry) and that the actual issue is that our DisplayGeometry is not aware of viewports. This, however, remains to be improved in future bugs, when someone actually has such a use case. For now, rendering a still image is fine (for me).

[cf263f]: Merge branch 'bug-13543-viewport-last-final'

Merged commits:

2013-07-10 17:54:10 Daniel Maleike [a626d0]
Imitate VTK viewport behavior in OpenGL mapper rendering

When VTK gets a viewport via vtkRenderer::SetViewport()
it scales the scene into this viewport. This commit makes
MITK's OpenGL based mappers bahave in the same way.

As commented in mitk::VtkPropRenderer, this is not an ideal
solution but a first fix. Plus, there are quite some tests
to save us from regressions.

[240f21]: COMP: Merge branch 'bug-13543-viewport-last-final'

Merged commits:

2013-07-12 09:37:40 Daniel Maleike [98d2b4]
Condition test execution

[86a2d6]: COMP: Merge branch 'bug-13543-viewport-last-final'

Merged commits:

2013-07-15 12:20:32 Daniel Maleike [ea801c]
Don't run tests in parallel

[5d1081]: COMP: Merge branch 'bug-13543-viewport-last-final'

Merged commits:

2013-07-16 11:52:11 Daniel Maleike [4fa685]
Exclude code if not BUILD_TESTING (produces CMake errors otherwise)

[e3dc28]: COMP: Merge branch 'bug-13543-viewport-last-final'

Merged commits:

2013-07-19 10:09:08 Daniel Maleike [f82275]
Link updated test data

All the previous commits were needed to make the rendering tests work. There were a couple of issues that I did not know about. The final commit added alternative reference images, which make the tests pass on a Windows continuous client, which has a slightly different anti-aliasing (or similar).

[a84ebc]: Merge branch 'bug-13543-viewport-last-final'

Merged commits:

2013-07-22 15:36:12 Daniel Maleike [ad602a]
COMP: yet another reference image

Forgot to close this issue, all is fixed, tests are working.