Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

albedo, illumination and normal renderer added #98

Merged
merged 18 commits into from
Apr 6, 2018

Conversation

BernhardEgger
Copy link
Member

Adding new renderer for:

  • surface normals
  • albedo
  • illumination

I'm not sure if the albedo and illumination should be rendered more elegant. I tried over PixelShaders but there seems to be a hurdle from the two illumination models which are slighty hardcoded.

@BernhardEgger
Copy link
Member Author

The updates calculates the normals not on the mesh but on the cameratransformed mesh (andreas, please check if I understood that correctly).

Code to test:

scalismo.initialize()

val model = MoMoIO.read(new File("/afs/csail.mit.edu/u/e/egger/unibas/model2015/model2012_l7_face12_to_bfm_pca.h5")).get

val renderer = MoMoRenderer(model)
val correspondenceRenderer = CorrespondenceMoMoRenderer(model)
val normalMapRenderer = NormalMapRenderer(correspondenceRenderer)
val depthRenderer = DepthMapRenderer(correspondenceRenderer)
val illRenderer = IlluminationRenderer(model)
val albedoRenderer = AlbedoRenderer(model)

val rps = RenderParameter.default.withMoMo(MoMoInstance(IndexedSeq(0.0),IndexedSeq(0.0),IndexedSeq(0.0), new URI(""))).withPose(pose = RenderParameter.default.pose.copy(yaw = 1.2)).withEnvironmentMap(RenderParameterIO.read(new File("/tmp/face_39343.rps")).get.environmentMap)

PixelImageIO.write(renderer.renderImage(rps), new File("/tmp/image.png"))
PixelImageIO.write(normalMapRenderer.renderImage(rps), new File("/tmp/normalMap.png"))
PixelImageIO.write(depthRenderer.renderImage(rps), new File("/tmp/depth.png"))
PixelImageIO.write(illRenderer.renderImage(rps), new File("/tmp/ill.png"))
PixelImageIO.write(albedoRenderer.renderImage(rps), new File("/tmp/albedo.png"))

@sschoenborn
Copy link
Contributor

Creating so many individual renderers might go a bit too far? PixelShader can be used relatively easily with rather general contents - but if you still want all the convenience renderers, do not forget about the ColorTransform in RenderParameter. It also influences the color in the image. And prefer the normalizing methods of RenderParameter, such as noLightAndColor etc., rather than defining your new (and incomplete) ad-hoc versions.

@BernhardEgger
Copy link
Member Author

An updated version uses the noLightAndColor method. Those methods where mainly added because I would like to add them to the parametric-face-image-generator. From feeback (in person and on the mailing-list) those renderers seem to be usually used in the community and could also be valuable for other projects.

@unibas-gravis unibas-gravis deleted a comment Mar 29, 2018
@andreas-schneider
Copy link

The albedo and the illumination renderer do not use the correspondence renderer is this on purpose?

}
}

normalMap.map(n =>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, what is done in this map can be done in the map above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maps merged

@BernhardEgger
Copy link
Member Author

I did not use the correspondence renderer since the mesh it gives is not the vertexcolormesh anymore. The color gets lost, no?

Copy link
Member

@Andreas-Forster Andreas-Forster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the need for generating all these images. However, I do not like the solution how it is implemented right now. But I have no satisfying solution right out of the box. Some questions are: What is the criterium to extend either from RenderFromCorrespondenceImage or from MoMoRenderer? Do we really need most of them being MoMoRenderer? Could we instead reduce them to be of type ParametricImageRenderer with corresponding type parameter? Do we need to factor out a common base type of these renderers from the MoMoRenderer?

Maybe it is a good idea to merge the pull request and open an Issue to discuss and refactor those renderers so that nobody is blocked until a final descision.

@BernhardEgger
Copy link
Member Author

For me it is hard to come up with a better solution. The concept of the renderers and the concepts on the mesh side additional to the triangle mesh itself are not very accessible for me - I still don't understand the underlying ideas.

@unibas-gravis unibas-gravis deleted a comment Apr 4, 2018
@Andreas-Forster
Copy link
Member

Andreas-Forster commented Apr 4, 2018

I just provided a different proposal for the implementation of the modality renderers. Note that the MoMoRenderer changes only in a way that no user of our lib should notice it. Some of the functionality is shifted into the ParametricModel class which is no longer a trait now.

The usage is given in the following code snippet. This snippet also shows that it is faster when we use the CorrespondenceRenderer for generating mutiple images. It might be even faster when rendering only one image. But this should be investigated further.

Please comment on this solution. We can also discard my proposal and switch back to the former request if you see any reason.

/* OUTDATED SEE NEXT COMMENT */

…function harder. The cause is that the correspondenceMoMoRenderer is also a ParametericModel which is required for the other type of functions.
@Andreas-Forster
Copy link
Member

Andreas-Forster commented Apr 4, 2018

Increased safety a bit by separating the functions. A cause for errors is that the functions which reside now in the ParametricModelModalityRenderer could take also a CorrespondenceMoMoRenderer as parameter. So it was not clear at call site which method was choosen given that no clear color was passed. Now only one of the functions should be in scope. The new example code is now:

import java.io.File
import java.net.URI

import scalismo.geometry.{Vector, _3D}
import scalismo.faces.color.{RGB, RGBA}
import scalismo.faces.gui.ImagePanel
import scalismo.faces.io.MoMoIO
import scalismo.faces.momo.MoMo
import scalismo.faces.parameters._
import scalismo.faces.sampling.face.{CorrespondenceMoMoRenderer, MoMoRenderer, ModalityRenderers}
import scalismo.faces.sampling.face.ModalityRenderers._
import scalismo.utils.Random

object Playground extends App {

  scalismo.initialize()
  implicit val rng = Random(32l)

  val modelFile: File = new File("/export/faces/model/model2017/model2017-1_face12_nomouth.h5")
  val model: MoMo = MoMoIO.read(modelFile).get
  val modelURI: URI = modelFile.toURI

  val parameter = RenderParameter.defaultSquare.
    withMoMo(MoMoInstance.fromCoefficients(model.sampleCoefficients(), modelURI)).
//    withEnvironmentMap(SphericalHarmonicsLight((0 until 9).map(_=>Vector(rng.scalaRandom.nextGaussian(),rng.scalaRandom.nextGaussian(),rng.scalaRandom.nextGaussian())))).
    withEnvironmentMap(SphericalHarmonicsLight(IndexedSeq[Vector[_3D]]())).withDirectionalLight(DirectionalLight(RGB(rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble()),RGB(rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble()),Vector(rng.scalaRandom.nextGaussian(),rng.scalaRandom.nextGaussian(),rng.scalaRandom.nextGaussian()),RGB(rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble()),shininess = 6.0)).
    withPose(Pose(0.95,Vector(-10,20,-1050),0.124,0.042,0.242)).
    withColorTransform(ColorTransform(RGB(rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble()),rng.scalaRandom.nextDouble(),RGB(rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble(),rng.scalaRandom.nextDouble())))

  val renderer = MoMoRenderer(model).cached(5)

  val ( image, normals, depth, albedo, illumination) = scalismo.utils.Benchmark.benchmark({
    val image = renderer.renderImage(parameter)
    val normals = ParametricModelModalityRenderer.renderNormals(parameter, renderer)
    val depth = ParametricModelModalityRenderer.renderDepthMap(parameter, renderer)
    val albedo = ParametricModelModalityRenderer.renderAlbedo(parameter, renderer)
    val illumination = ParametricModelModalityRenderer.renderIllumination(parameter, renderer)
    (image, normals, depth, albedo, illumination)
  }, "momo renderer")


  val crRenderer = CorrespondenceMoMoRenderer(model).cached(10)

  val ( imageCR, normalsCR, depthCR, albedoCR, illuminationCR) = scalismo.utils.Benchmark.benchmark( {
    val image = crRenderer.renderImage(parameter)
    val normals = CorrespondenceMoMoModalityRenderer.renderNormals(parameter, crRenderer)
    val depth = CorrespondenceMoMoModalityRenderer.renderDepthMap(parameter, crRenderer)
    val albedo = CorrespondenceMoMoModalityRenderer.renderAlbedo(parameter, crRenderer)
    val illumination = CorrespondenceMoMoModalityRenderer.renderIllumination(parameter, crRenderer)
    (image, normals, depth, albedo, illumination)
  }, "correspondence renderer")



  import scalismo.faces.gui.GUIBlock._

  ImagePanel(image).displayIn("image")
  ImagePanel(imageCR).displayIn("imageCR")

  ImagePanel(colorNormalImage(normals,RGBA.BlackTransparent)).displayIn("normals colored")
  ImagePanel(colorNormalImage(normalsCR,RGBA.BlackTransparent)).displayIn("normalsCR colored")

  ImagePanel(normalizeDepthMapIamge(depth,0.0)).displayIn("depth normalized")
  ImagePanel(normalizeDepthMapIamge(depthCR,0.0)).displayIn("depthCR normalized")

  ImagePanel(albedo).displayIn("albedo")
  ImagePanel(albedoCR).displayIn("albedoCR")

  ImagePanel(illumination).displayIn("illumination")
  ImagePanel(illuminationCR).displayIn("illuminationCR")
}

@BernhardEgger
Copy link
Member Author

For me the the correspondenceRenderer and the ParametricModel looks like a lot of code duplication now - is this necessary?

Would it make sense to have it as a part of the standard MoMoRenderer? It already has renderMesh and renderImage?
I renamed some of the functions to make clear what it renders. the renderIllumination does not really render the illumination but an image-based representation (renderIlluminationImage or renderIlluminationVisualization - whatever you prefer).
The same is true for the renderNormals - I added a renderNormalImage or renderNormalVisualization)

No every function call needs the correspondencerenderer, this is not consistent with all other rendering calls - shouldn't this be a member (or if added to the MoMoRenderer this would also disappear)

@sschoenborn
Copy link
Contributor

I do not know how much I should still interfere here .. Just my comments on your proposals:

The original proposal of Bernhard at the beginning had some simplicity appeal to it: there is the collection of various renderers, each with its own class. All of them are actually ParametricImageRenderers. However, I do not like the implementation too much. You should think about whether you really want all that inheritance -- Is a IlluminationRenderer still a valid MoMoRenderer? I would say no; the original design idea was to have the traits Parametric*Renderer as the generic parts and have MoMoRenderer a closed implementation [it is not really sealed for the caching trick to work]. You can add more renderers anytime by just implementing the respective traits (ParametricImageRenderer in these cases). MoMoRenderer should be sealed in my opinion - but you can use composition to avoid duplication of course.

The second approach with CorrespondenceMoMoModalityRenderer, I prefer from the implementation point of view. But it lacks the elegance of the common ParametricImageRenderer trait - all the individual modalities need to be called as a specific method. And I do not understand why both versions, CorrespondenceMoMoModalityRenderer (a really long name :)) and the other similar renderer, are required.

Additional remark: colorNormalImage etc. are implemented as direct duplicates, once with Option and once without. This can be done much simpler and cleaner by just mapping a pattern match over the first result (or a _.getOrElse(clearColor)).

I will not use these classes anytime soon and have not participated in architecture discussions for a long time now - so feel free to ignore my comments completely :)

@Andreas-Forster
Copy link
Member

Sandro, Your opinion about a PR is always welcome in our discussions. But we might not always have enough time or foresight to follow all inputs ;-)

Based on your comments I have another proposal which addresses parts of your comments.

We could merge both approaches somehow and make the modality renderers pure ParametricImageRenderers. In the beginning, I just did not like the fact that they were extending the MoMoRenderer.

The second version (I removed it for now, but can add it if we think it might be useful.) was because it had a much simpler parameter, a ParametricModel and in addition, it was faster when generating only a single image modality. I left out this "simpler" version for now.

I changed now the colorNormalImage to always require a clear color. The proposition to call one method from the other one would slow down the method by nearly a third because it would iterate twice over the image.

Keeping everything at a single location in our library should make it enough simply to remove or change it in the future if we change our mind.

@sschoenborn
Copy link
Contributor

👍

@Andreas-Forster Andreas-Forster merged commit b699aa1 into master Apr 6, 2018
@Andreas-Forster Andreas-Forster deleted the additionalRenderers branch April 6, 2018 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants