Hi Robert,

thanks for the commit. On osgviewer --run-on-demand runs quite well, even when 
I am moving some small window over it - osgviewer is repainted correctly.

Concerning local display variable, I considered using display and _display  
with complete different  content that user must not mix - it is too easy to 
overlook by other programmers and to misuse / mix them, resulting in very hard 
to find bugs. Thus, I explicitly changed the name to be clear about the 
existence of the two X-server connections that the programmer have to use 
correctly.

My personal preference would be to change display to _eventDisplay (as 
suggested by submission), but I am fine with both options. However, there are 
still two bugs in GraphicsWindowX11.cpp that you did not merge. Please, 
consider them once again:

- there are missing break (in switch statement). It is in the 
GraphicsWindowX11::checkEvents() on lines 1178 and 1525, when processing 
ClientMessage. This is dangerous particularly in the first case as EXPOSE can 
be executed upon receiving DELETE_WINDOW and other client messages. In the 
second case, it will help in the case that someone extends the switch in the 
future.

- missing OSGVIEWER_EXPORT for graphicswindow_X11(), currently line 2116.
The same problem exists for graphicswindow_Win32() on line 2696 in 
GraphicsWindowWin32.cpp, making it not possible to call the funtion from the 
application code through OSG DLL api. Please, change both (soorry for not 
sending you GraphicsWindowWin32.cpp as I have more local changes inside it 
that are not ready for submission.

Thanks,
John

> ------------ Original message ------------
> From:  Robert Osfield <[email protected]>
> Subject:  Re: [osg-submissions] [osg-users] requestRedraw bug during 
ON_DEMAND rendering
> Date:  19. 4. 2011  14:05:30
> ----------------------------------------
> Hi John,
> 
> I've just reviewed your changes and the look sensible.  The only part
> I didn't merge was the change from using the local display variable in
> the GraphicsWindowX11::checkEvents() method, as the local display
> variable is already set to _eventDisplay so there isn't any need to
> change instances of display to _eventDisplay.  I do wonder if you
> misread the implementation, inpreteting the display as _display.
> 
> I haven't test the on demand side yet, but the normal usage of the OSG
> looks to be working fine so I've gone ahead and checked the changes
> in.  Could you check svn trunk to make sure that things are working
> fine for you.
> 
> Cheers,
> Robert.
> 
> On Thu, Mar 17, 2011 at 3:13 PM, PC John <[email protected]> wrote:
> > Hi Robert,
> > please find attached proposed fix for ON_DEMAND rendering. The biggest
> > issue was that the windows did not act on repaint request (WM_PAINT,
> > EXPOSE,...)
> > 
> > Detailed explanation:
> > - I implemented requestRedraw using the push approach (not using
> > GraphicsWindow::_requestRedraw flag that I was considering) as there may
> > be multiple viewers reading the flag and fighting to reset it after the
> > paint request, while some viewers may not spot the request to redraw
> > - I made windows call GraphicsWindow::requestRedraw when they receive
> > appropriate message (WM_PAINT, EXPOSE, RESIZE,...)
> > - There were issues on Linux that windows did not want to close using x
> > button. Resolved by moving the test for DeleteWindow event from
> > swapBuffersImplementation() to GraphicsWindowX11::checkEvents(). The
> > difficulty was that DeleteWindow event is not coming using
> > _eventDisplay, but through _display.
> > - The last difficulty was that it is necessary to call
> > ViewerBase::checkWindowStatus() to set _done to true when all windows are
> > closed. This did not happened recently in ON_DEMAND run scheme. I put the
> > call to checkWindowStatus() to eventTraversal.
> > 
> > Feel free to adjust these or to further discuss.
> > John
> > 
> >> ------------ Original message ------------
> >> From:  Robert Osfield <[email protected]>
> >> Subject:  Re: [osg-users] requestRedraw bug during ON_DEMAND rendering
> >> Date:  14. 3. 2011  18:13:16
> >> ----------------------------------------
> >> Hi John,
> >> 
> >> Interesting problem you've unconvered.   Responding to windowing
> >> redraw events is something it should do, but alas it wasn't something
> >> I thought about nor tested on writing the feature.  I'm rather rusty
> >> on the topic so if you have looked into the code and come up with a
> >> possible solution you are further ahead than me, so I'd suggest just
> >> going with what you feel would work and the post the code to be
> >> review, and on review I'll try and get my head around this topic.
> >> 
> >> Cheers,
> >> Robert.
> >> 
> >> On Mon, Mar 14, 2011 at 4:17 PM, PC John <[email protected]> wrote:
> >> > Hi all,
> >> > 
> >> > I am using osgViewer::Viewer in ON_DEMAND RunFrameScheme, e.g. there
> >> > is no continual re-rendering and 100% CPU loading, but the scene is
> >> > rendered only when really needed.
> >> > 
> >> > Bug: The window is not repainted when obscured by another window and
> >> > re-shown again.
> >> > 
> >> > I was trying to fix the problem and these are my findings:
> >> > - when window is damaged and needs to be repainted, it receives
> >> > WM_PAINT (Win32) or Expose (Linux) message, or paintEvent() is called
> >> > on Qt. I tried to call requestRedraw() inside these, but it is just
> >> > an empty function inside osgViewer::GraphicsWindow. Is it a bug,
> >> > intended behaviour or unimplemented functionality? In the first and
> >> > third case, I am going to contribute the missed functionality by
> >> > testing all windows from
> >> > osgViewer::Viewer::checkNeedToDoFrame().
> >> > 
> >> > John
> >> > _______________________________________________
> >> > osg-users mailing list
> >> > [email protected]
> >> > http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.
> >> > org
> >> 
> >> _______________________________________________
> >> osg-users mailing list
> >> [email protected]
> >> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.or
> >> g
> > 
> > _______________________________________________
> > osg-submissions mailing list
> > [email protected]
> > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegra
> > ph.org
> 
> _______________________________________________
> osg-submissions mailing list
> [email protected]
> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph
> .org
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org

Reply via email to