Hi Paul,

On Tue, Nov 10, 2009 at 10:46 PM, Paul Martz <[email protected]> wrote:
> Hi Robert -- For your convenience, I've attached the files you missed from
> my previous submission.

Thanks.  I've now done a review and have decided to amend the settings
in osg::GraphicsContext::Traits a bit to remove there specificity to
GL3, and changed the way the the defaults are set up.   The new
members of Traits are:

            // settings used in set up of graphics context, only
presently used by GL3 build of OSG.
            std::string     glContextVersion;
            unsigned int    glContextFlags;
            unsigned int    glContextProfileMask;

Traits also now takes a DisplaySettings pointer in it's constructor,
which allows us to move handling of DisplaySettings directly into
GraphicsContext.cpp rather than have it spread out through the Traits
set up in src/osgViewer/View.cpp.  The new code in GraphicsContext.cpp
looks like:


GraphicsContext::Traits::Traits(DisplaySettings* ds):
            x(0),
            y(0),
            width(0),
            height(0),
            windowDecoration(false),
            supportsResize(true),
            red(8),
            blue(8),
            green(8),
            alpha(0),
            depth(24),
            stencil(0),
            sampleBuffers(0),
            samples(0),
            pbuffer(false),
            quadBufferStereo(false),
            doubleBuffer(false),
            target(0),
            format(0),
            level(0),
            face(0),
            mipMapGeneration(false),
            vsync(true),
            useMultiThreadedOpenGLEngine(false),
            useCursor(true),
            glContextVersion("1.0"),
            glContextFlags(0),
            glContextProfileMask(0),
            sharedContext(0),
            setInheritedWindowPixelFormat(false),
            overrideRedirect(false)
{
    if (ds)
    {
        alpha = ds->getMinimumNumAlphaBits();
        stencil = ds->getMinimumNumStencilBits();
        sampleBuffers = ds->getMultiSamples();
        samples = ds->getNumMultiSamples();
        if (ds->getStereo())
        {
            switch(ds->getStereoMode())
            {
                case(osg::DisplaySettings::QUAD_BUFFER):
quadBufferStereo = true; break;
                case(osg::DisplaySettings::VERTICAL_INTERLACE):
                case(osg::DisplaySettings::CHECKERBOARD):
                case(osg::DisplaySettings::HORIZONTAL_INTERLACE):
stencil = 8; break;
                default: break;
            }
        }

        glContextVersion = ds->getGLContextVersion();
        glContextFlags = ds->getGLContextFlags();
        glContextProfileMask = ds->getGLContextProfileMask();
    }
}

Note, the default values are the same as for the wgl_ and glx_
extensions specify.  I've also opted for a std::string for the version
to allow us to potentially encode other info into the string rather
than just major/minor versions.  The introduction of DisplaySetttings
means that it'll now take over the env var passing for the defaults,
and also allow command line parsing to override the env vars and
defaults.  This will allow us to do:

  osgviewer cow.osg --gl-version 3.2 --gl-flags 2 --gl-profile-mask 1

The env vars that are now picked up are OSG_GL_CONTEXT_VERSION,
OSG_GL_CONTEXT_FLAGS and OSG_GL_CONTEXT_PROFILE_MASK, as well as
OSG_GL_VERSION which maps to OSG_GL_CONTEXT_VERSION to make it a
little less verbose.   Perhaps we should drop this mapping, perhaps we
should drop the _CONTEXT_, I have no strong opinions either way.

> I did not review all of your changes (I don't think that's necessary on a
> development branch, we'll sort out problems as they arise), but did look at
> FrameBufferObject.cpp and noted that you didn't account for the presence of
> packed depth stencil in GL3, even though that was correctly handled in my
> original submission. In the attachment, I've included a fix for this.

I've merged and checked into svn/trunk the pack depth stencil change now.

I'll hold back from merging the changes to GraphicsWindowWin32 as my
changes to Traits will have broken the build on these.  I could have a
guess at fixing them, but am not comfortable with not being able to
compile test these changes.   I also have  query about one of the
changes to GraphicsWindowWin32.cpp, could you explain why you removed
the ResotreContext code block from bool
GraphicsWindowWin32::realizeImplementation():

                struct RestoreContext
                {
                    RestoreContext()
                    {
                        _hdc = wglGetCurrentDC();
                        _hglrc = wglGetCurrentContext();
                    }
                    ~RestoreContext()
                    {
                        if (_hdc)
                        {
                            wglMakeCurrent(_hdc,_hglrc);
                        }
                    }
                protected:
                    HDC        _hdc;
                    HGLRC    _hglrc;
                } restoreContext;


The reason why I'm perplexed by the removal of this code is that there
must have been a reason for the code in the first place, it seems odd
that this reason has just disappeared.  I don't have any expertise or
experience with the GraphicsWindowWin32 so I have defer to experts
like yourself.

Thanks for any clarification you can provide.

Robert.
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org

Reply via email to