Hi John, On Wed, Apr 20, 2011 at 10:00 AM, PC John <[email protected]> wrote: > 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.
Excellent news. > 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. Hardwiring to _eventDisplay is not entirely safely as it depends upon which thread the calling code is from as to which display variable to use, so being able to dynamically assign this is best, so a local variable provides this flexibility. Using a more specific local variable name might be better though as _display and display are rather close and could easily be confused. > 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. I've just applied the additional of the break and will check in soon. > - 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. Why is the export required for graphicswindow_X11()? Are you building X11 under Windows? What is your suggestion for synatax? I don't want to add something that might not work under Windows as no matter what I add under Linux it'll just disappear once the macro is expanded. Thanks, Robert. _______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
