I wrote a core change request.

I like the idea of textures much, but the change request reads like there are more details to be figured out. I suggest to improve the plan, taking into account the experiences from the long-running T10174, T8165, and T13543. Texture mapping was much trouble in T10174 and it would be good to have a common plan of how we "do rendering".

I'm sorry. I thought the idea about core change requests was to write them as early as possible to figure out whether implementation of a certain feature makes sense in the core or not.

You told me once that I should not request the flag when all the work is done and a detailed plan (which I agree is definitely necessary here) belongs to the actual work (for me at least).

We should really separate this. In my opinion, there are 3 main issues we want to cover with just one flag:

1. Does it make sense to implement a certain feature in the core? (rejection e.g. is it already somewhere else, we don't need it, etc.)
2. Is it well planed? (Did we investigate different strategies, can we use code from another toolkit, are all use cases covered etc.)
3. Code reviews. (Is there enough documentation, is the code well written, what about the coding style etc.?)

In my opinion our current process does not reflect this properly and it feels frustrating to have this flag denied. In fact, we should make different stages with different outcome possibilities. We could make it like the review process of a journal paper ;), if human resources wasn't an issues ...

New remote branch pushed: bug-13956-FirstTextureProperty

I talked to Markus and we both think a property would be the best way to go. I pushed a branch with a first approach. The diff looks quite horrible, but it is just because I reformatted the whole surface mapper file. I only added this code and the corresponding includes:

mitk::SmartPointerProperty::Pointer imagetextureProp;
imagetextureProp = dynamic_cast< mitk::SmartPointerProperty * >(
            GetDataNode()->GetProperty("Surface.Texture", renderer));
if(imagetextureProp.IsNotNull())
{
    mitk::Image* miktTexture = dynamic_cast< mitk::Image* >( imagetextureProp->GetSmartPointer().GetPointer() );

    if(miktTexture->GetDimension(2) > 2)
    {
        MITK_WARN << "3D Textures are not supported by VTK and MITK. The first slice of the volume will be used instead!";
    }

    mitk::ImageSliceSelector::Pointer sliceselector = mitk::ImageSliceSelector::New();
    sliceselector->SetSliceNr(0);
    sliceselector->SetChannelNr(0);
    sliceselector->SetTimeNr(0);
    sliceselector->SetInput(miktTexture);
    sliceselector->Update();

    vtkSmartPointer<vtkTexture> vtkTxture = vtkSmartPointer<vtkTexture>::New();
    vtkTxture->SetInput(sliceselector->GetOutput()->GetVtkImageData());

    ls->m_Actor->SetTexture(vtkTxture);
}

The idea is:
-Use a smart-pointer property to pass an arbitrary mitkImage to the SurfaceVtkMapper3D
-If this property is set, the SurfaceVtkMapper3D uses this image as texture

Open issues are:
-How are the texture coordinates generated? Right now, the user has to provide them and add them to the polydata (common in VTK). We could offer some "default" texture coordinates, but in most cases they would not make sense.
-What happens with 3D images? VTK allows only for 2D textures. Currently, I used the first slice of a potential 3D image.
-Saving/Loading of this property has to be checked in detail.

@Daniel: I assume the 3 mentioned bugs were much trouble, because we moved far away from the common VTK use cases and wrote our own rendering code. I already mentioned my concerns regarding that. I think our "common plan" is to use as much VTK as possible and this bug here is pretty straight forward VTK code :).

Regarding last issues:

-How are the texture coordinates generated?
-> This is the user's job! If we assume anything about the texture or the surface (like 1:1 mapping is possible), the results are horrible. An error message will be generated if the user does not provide any texture coordinates.

-What happens with 3D images?
->The first slice should be used and a warning should be generated.

-Saving/Loading of this property has to be checked in detail.
-> This seems to be a bug of the SmartPointerProperty in general => new T14196

To be done:
Testing of the Property

Example Screenshot of the property: Two texture liver phantoms

I added two rendering tests:
SurfaceVtkMapper3DTest renders a surface and puts a texture on it. Texture coordinates are mapped 1 to 1.
SurfaceVtkMapper3DTexturedSphereTest renders a sphere generated by VTK and puts a texture on it.

I also added test data to the MITK-Data repository.

Rewrote the core change request.

Sounds good to me

[69512d]: Merge branch 'bug-13956-FirstTextureProperty'

Merged commits:

2013-01-11 16:39:43 Thomas Kilgus [347db2]
New MITK-Data revision tag.

2013-01-11 16:35:55 Thomas Kilgus [488d95]
Added error message when no TCoors are set. Last cosmetic changes.

2013-01-11 14:35:33 Thomas Kilgus [e2915e]
Added two tests for surface texturing and enhanced the renderingTestHelper to handle 3D render windows.

2013-01-08 09:59:52 Thomas Kilgus [c71418]
First approach for a texture property. Does not concern all use cases, yet, but works in principle.

I merged the version with the SmartPointerProperty, as a first try.

Todo: Take care of loading and saving via replacing the image with a DataNode or implementing a new mitkTextureProperty.

[35b781]: Merge branch 'bug-13956-FirstTextureProperty'

Merged commits:

2013-01-16 16:48:19 Thomas Kilgus [0f4031]
COMP: Fixed reorient issues in swivel test. Outcommented texture tests, until they work in windows.

[bc5550]: Merge branch 'bug-13956-FirstTextureProperty'

Merged commits:

2013-01-16 16:57:42 Thomas Kilgus [4854ed]
COMP: Forgot to outcomment two bracers.

[31641d]: Merge branch 'bug-13956-FirstTextureProperty'

Merged commits:

2013-01-16 18:00:54 Thomas Kilgus [d3bf4d]
COMP: Trying to reactivate tests.

[08b807]: Merge branch 'bug-13956-FirstTextureProperty'

Merged commits:

2013-01-16 20:44:00 Thomas Kilgus [cf5bfb]
COMP: Tests still no running under windows...

[6ced64]: Merge branch 'bug-13956-FirstTextureProperty'

Merged commits:

2013-01-21 16:49:41 Thomas Kilgus [05c0f9]
Trying to reactivate tests. Added RUN_SERIAL Property.

[0e5943]: Merge branch 'bug-13956-FirstTextureProperty'

Merged commits:

2013-01-21 17:31:39 Thomas Kilgus [e5cc34]
COMP: Removed one failing test. At least one test is running now.

Most of this bug is fixed except for one issue:

The property is a SmartPointerProperty which cannot be saved/loaded.

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

This bug can be closed now.

The last issue is discussed in T15936.