Changeset 10804 for OpenSceneGraph/trunk/src/osg/GraphicsContext.cpp

Timestamp:

11/20/09 12:39:10 (4 years ago)

Author:

robert

Message:

From Himar Carmona, " this submission resolves an issue regarding the setup of a slave camera with an (incorrect) coding style. Sharing the same instance of osg::Viewport with a camera and a slave camera causes incorrect rescaling in GraphicsContext::resizedImplementation, due to the viewport being rescaled twice (once per camera). Though viewports sharing is not intentional, it can be done with the current version of OSG and be a potential pitfall for anyone.

As Robert pointed out, i opted for modifying the resize code where the break is to minimize code changes, avoiding the duplicate resize of the viewport with the use of a vector and a search for duplicates. Not very elegant (avoiding an effect of a cause), another approach could be ripping out the method osg::Camera::setViewport(osg::Viewport*) which is more inline with Roberts rationale behind not to share viewports between cameras and left only its overloaded method setViewport(x,y,width,height). But this approach need some refactoring due to the intense use of the method. Notice also that the resize works well without this change if no sharing occurs, and the user of the method can opt for always call setViewport with a new instance.

"

Note from Robert Osfield, changed this submission to use an std::set<Viewport*> rather than an std::vector<> as it keeps the code a bit cleaner and more compact.

Files:

• OpenSceneGraph/trunk/src/osg/GraphicsContext.cpp (modified) (3 diffs)

OpenSceneGraph/trunk/src/osg/GraphicsContext.cpp

@@ -844 +844 @@
 void GraphicsContext::resizedImplementation(int x, int y, int width, int height)
 {
+    std::set<osg::Viewport*> processedViewports;
+
     if (!_traits) return;
     
@@ -850 +852 @@
     double aspectRatioChange = widthChangeRatio / heigtChangeRatio; 
     
+
     for(Cameras::iterator itr = _cameras.begin();
         itr != _cameras.end();
@@ -862 +865 @@
         if (viewport)
         {
-            if (viewport->x()==0 && viewport->y()==0 &&
-                viewport->width()>=_traits->width && viewport->height()>=_traits->height)
+            // avoid processing a shared viewport twice
+            if (processedViewports.count(viewport)==0)
             {
-                viewport->setViewport(0,0,width,height);
-            }
-            else
-            {
-                viewport->x() = static_cast<osg::Viewport::value_type>(double(viewport->x())*widthChangeRatio);
-                viewport->y() = static_cast<osg::Viewport::value_type>(double(viewport->y())*heigtChangeRatio);
-                viewport->width() = static_cast<osg::Viewport::value_type>(double(viewport->width())*widthChangeRatio);
-                viewport->height() = static_cast<osg::Viewport::value_type>(double(viewport->height())*heigtChangeRatio);
+                processedViewports.insert(viewport);
+
+                if (viewport->x()==0 && viewport->y()==0 &&
+                    viewport->width()>=_traits->width && viewport->height()>=_traits->height)
+                {
+                    viewport->setViewport(0,0,width,height);
+                }
+                else
+                {
+                    viewport->x() = static_cast<osg::Viewport::value_type>(double(viewport->x())*widthChangeRatio);
+                    viewport->y() = static_cast<osg::Viewport::value_type>(double(viewport->y())*heigtChangeRatio);
+                    viewport->width() = static_cast<osg::Viewport::value_type>(double(viewport->width())*widthChangeRatio);
+                    viewport->height() = static_cast<osg::Viewport::value_type>(double(viewport->height())*heigtChangeRatio);
+                }
             }
         }