RenderKit / ospray

An Open, Scalable, Portable, Ray Tracing Based Rendering Engine for High-Fidelity Visualization
http://ospray.org
Apache License 2.0
1.02k stars 186 forks source link

restored height fix for orthographic camera #291

Closed pnav closed 5 years ago

pnav commented 5 years ago

This was a fix I pushed back in April and it appears it was lost in the scenegraph overhaul. Please restore it. Thanks!

johguenther commented 5 years ago

I change the logic for the orthographic camera in a01611817ca63e4b0a947a9decd770eb170f5656 to relate its height to the world bounding box, which I think is a better heuristic than using the framebuffer size.

pnav commented 5 years ago

Thank you for the reply, Johannes. I would like to know more about why you believe the world bounding box to be better. For my use case, scientific visualization where the world extent corresponds to the data extent, using the world bounding box results in an image overly-zoomed on the data center. Also, it appears that the height parameter is no longer exposed, so that there is no way to adjust this through the camera. I would need to add a fake bounding box around the world data, and it seems wrong (at least to me) to have to change the world construction to impact the camera behavior. It would be better for camera behavior to be contained in the camera and its by-products.

As you rightly imply, using image height is merely a different heuristic, but it is (1) a more common expression of orthographic camera construction in my experience, (2) though not directly contained in the camera, is more closely related to it than a world-level parameter, and (3) explicitly exposes the parameter so that the user can adjust it if needed.

What is really needed, I suppose, is a 'zoom' camera parameter that allows the user to adjust the height of the orthographic camera regardless of the initial value provided by OSPRay. I can look into adding this.

johguenther commented 5 years ago

I see the problem of overly-zoomed in: the factor 0.5 in OSPApp.cpp:307 is too small, 0.8 or 1.0 is better (I will change that).

The height parameter of the orthographic camera is a world-space quantity, thus it should work reasonably well to use the scene bounding box per default.

How do you switch to the orthographic camera? If I change the default camera in SceneGraph.cpp:27 then height is exposed as parameter (instead of fovy) and one can change the height/"zoom" while the app is running.

pnav commented 5 years ago

Thanks Johannes. I'm trying to adapt the ExampleViewer code to act as a viewer for my renderer. If I hard-code an Orthographic camera in OSPApp as you suggest, it works, but I would like my code to be self-contained in an OSPRay module so it can be deployed, updated, and benefit from OSPRay updates without hard-coded hacks in OSPRay classes.

Using ExampleViewer as a template, I've modified the render() method to whitelist my renderer and to swap to the orthographic camera. The renderer mods are successful, and I can use it with the perspective camera (though with odd results since the technique assumes an orthographic camera). However, adding the orthographic camera as I've done below actually segfaults, though I'm not yet sure why.

Any insights or suggestions you could provide are most welcome!

    void MyViewer::render(const std::shared_ptr<sg::Frame> &root)
    {    
      // add myrenderer to OSPApp whitelist
      std::vector<std::string> globalWhitelist = ospray::sg::Renderer::globalRendererTypeWhiteList();
      globalWhitelist.push_back(std::string("myrenderer"));
      ospray::sg::Renderer::setGlobalRendererTypeWhiteList(globalWhitelist);

      // update Renderer whitelist, since init from global whitelist at Renderer constructor 
      std::vector<sg::Any> whitelist;
      std::copy(globalWhitelist.begin(),
                globalWhitelist.end(),
                std::back_inserter(whitelist));
      root->child("renderer").child("rendererType").setWhiteList(whitelist);

      // add myrender options to imGUI interface
      root->child("renderer").child("rendererType").setValue(std::string("myrenderer"));

      #if 0  // if enabled, segfaults on ortho camera. Works with default perspective camera
      root->remove("camera");
      root->createChild("camera","OrthographicCamera");
      // init the ortho camera with fake up, pos, gaze params to make ImGuiViewer init happy
      // using the default values from OSPApp.h
      root->child("camera").createChild("up","vec3f",vec3f(0,1,0));
      root->child("camera").createChild("pos","vec3f",vec3f(0,0,-1));
      root->child("camera").createChild("gaze","vec3f",vec3f(0,0,0));

      // set the height to match the default frame height (see OSPApp.h)
      //root->child("camera").child("height").setValue((float)this->height);
      #endif

      ospray::ImGuiViewer window(root);

      window.create("My Viewer App",
                    fullscreen, vec2i(width, height));

      if (motionSpeed > 0.f)
        window.setMotionSpeed(motionSpeed);

      if (!initialTextForNodeSearch.empty())
        window.setInitialSearchBoxText(initialTextForNodeSearch);

      imgui3D::run();
    }
pnav commented 5 years ago

Hi Johannes,

After a bit more debugging effort, it looks like I have the Orthographic camera working as expected. The height field is indeed exposed, and I can set it in my code to a better value for my data (when it gets initialized in createChild() it gets set to 1.f so I need to update it anyway).
Here is the improved code from the #def block from above, the rest is the same:

#if 1
      auto &camera = root->child("camera");
      vec3f up = camera["up"].valueAs<vec3f>();
      vec3f pos = camera["pos"].valueAs<vec3f>();
      vec3f dir = camera["dir"].valueAs<vec3f>();
      vec3f gaze = camera.hasChild("gaze") ? camera["gaze"].valueAs<vec3f>() : vec3f(0.f);
      root->remove("camera");
      root->createChild("camera","OrthographicCamera");
      root->child("camera").child("up").setValue<vec3f>(up);
      root->child("camera").child("pos").setValue<vec3f>(pos);
      root->child("camera").child("dir").setValue<vec3f>(dir);
      root->child("camera").createChild("gaze","vec3f",gaze);

      // create a reasonable initial height setting
      root->child("camera").child("height").setValue<float>(768.f);

      root->commit();
#endif  

I will close this pull request as it appears unnecessary. Thanks for your help!