unibas-gravis / scalismo-faces

Scalable Image Analysis and Shape Modelling: Module to work with 2d images, with a focus on face images
Apache License 2.0
118 stars 27 forks source link

Fixed wrong marginal function. #103

Closed thogerig closed 6 years ago

thogerig commented 6 years ago

The marginal function to mask a MoMo gave wrong results. The reason was that the masked mesh can result in a different amount of PointIds. Since the mask MoMo function depends on the mesh in contrast to the marginal function for the GP, I would propose to rename it to mask().

BernhardEgger commented 6 years ago

See https://github.com/unibas-gravis/scalismo-faces/pull/95 for previous discussions

thogerig commented 6 years ago

I like the second idea also the most. How about passing the MeshCompactifier instead of a PointId list?

  def mask(originalMomo: MoMo, reducedMesh: MeshCompactifier): MoMo = {

      val maskedMesh = reducedMesh.transformedMesh
      val remainingPtIds = maskedMesh.pointSet.pointIds.map(id => reducedMesh.pointBackMap(id)).toIndexedSeq
      val maskedModelShape = originalMomo.neutralModel.shape.marginal(remainingPtIds)
      val maskedModelColor = originalMomo.neutralModel.color.marginal(remainingPtIds)
      MoMo(maskedMesh, maskedModelShape, maskedModelColor)
...

This is, however also not completely clean because the mesh given to the MeshCompactifier must be the models reference mesh.

thogerig commented 6 years ago

After some discussions, here a revised implementation:

What do you think of this change?

Andreas-Forster commented 6 years ago

Thank you for your update. I think it might be a good idea to make the things that can go wrong more transparent and provide the user with feedback what went wrong. So I encapsulated the return type in a Try and provided feedback in erroneous situations. What do you think about this?

Andreas-Forster commented 6 years ago

I added some tests and fixed a bug I discovered while developing the tests.

thogerig commented 6 years ago

Thanks for the additions and the bug fix. From my side the changes are fine.