On Wed, May 11, 2016 at 10:45:49PM -0400, David T. Lewis wrote:
> On Wed, May 11, 2016 at 03:35:49PM +0200, Bert Freudenberg wrote:
> > On 11.05.2016, at 15:02, David T. Lewis <lewis at mail.msen.com> wrote:
> > > 
> > > On Wed, May 11, 2016 at 10:47:11AM +0200, Bert Freudenberg wrote:
> > >> On 11.05.2016, at 07:32, tim Rowledge <tim at rowledge.org> wrote:
> > >> 
> > >>>> 
> > >>>> But this cannot possibly work. The window index used by HostWindowPlugin
> > >>>> is supposed to be opaque value, known to the plugin but not to the image.
> > >>>> On a Windows platform, the window index might be 1.
> > >>> 
> > >>> IIRC (after all, I wrote this 12 years ago) we defined that the ???root??? window had to have a 1 index just so we could rely upon something.
> > >> 
> > >> Yep. That's why it's hard-coded in DisplayScreen but a variable in HostWindowProxy.
> > >> 
> > > 
> > > Hmmm... it looks to me like it may take some work to make the unix
> > > plugin work that way. But maybe I was just overlooking something.
> > 
> > The version I see here actually works *only* for the main Squeak window:
> > 
> > static int display_hostWindowSetTitle(int windowIndex, char *newTitle, int sizeOfTitle)
> > { 
> >   if (windowIndex != 1)
> >     return -1;
> > 
> >   XChangeProperty(stDisplay, stParent,
> > 		  XInternAtom(stDisplay, "_NET_WM_NAME", False),
> > 		  XInternAtom(stDisplay, "UTF8_STRING",  False),
> > 		  8, PropModeReplace, newTitle, sizeOfTitle);
> > 
> >   return 0;
> > }
> > 
> > (and setting the size is not implemented)
> > 
> > static int display_hostWindowSetSize(int windowIndex, int w, int h)
> > { return -1; }
> > 
> 
> Aha, that gives me an idea. If the established convention is that window
> handle 1 refers to the Squeak display window, then maybe we can maintain
> the convention by having sqUnixX11.c check for this and convert it to the
> actual X11 Window of the display window.
> 
> The code that you quote above is the current SVN trunk. The oscog branch
> has the same implementation for display_hostWindowSetTitle, and adds various
> things from Qwaq including a display_hostWindowSetSize(). The Qwaq methods
> all to use the X11 Window as the handle, and that is the part that does
> not yet work.
> 
> So maybe there is an easy fix, convert index 1 to the window handle
> of the display window, stParent. For all other cases, treat the handles
> as opaque values. Yes, I think it will work :-)
> 

Sorry for replying to my own post, but I found one more "aha".

I now know that the original intent of the window handles for the host
window plugin included an assumption that handle 1 should always refer
to the main Squeak display. But I also know from Andreas' previous work
that any time the VM (a plugin) provides a "handle" reference to something
internal to that plugin, that it was very important that the handle be
treated as an opaque reference. In other words, the image should not be
able to decode that handle to figure out what was going on internally
within the plugin (as I routinely did with e.g. UnixOSProcessPlugin, to
Andreas' great dismay).

So, based on that background, I was expecting that the host window
handles in HostWindowPlugin would be treated in the same manner, and
that is why I was surprised to find that DisplayScreen class>>hostWindowIndex
was hard coded to answer 1, when I might have expected that it should
instead ask the VM for an opaque handle to the Squeak display window.

<OT>I came up with a workaround for the X11 display module that simply
maps host window handle 1 to the actual display window. That works
very well, and I can now resize the display window on a unix VM.
Based on that, I put an update in the inbox, which should be good
independent of this discussion. </OT>

I then took a look at some of the other Qwaq updates to sqUnixX11.c,
and I see that there are more recent updates that (AFIK) are not yet
in use in the Squeak VMs. These are identified by a version level
in vm/SqDisplay.h (SqDisplayVersionMinor >= 3). One of the newer
methods is:

  void *display_ioGetWindowHandle() { return (void *)stParent; }

This is exactly what I would have expected. stParent is the address of
a Window struct for the X11 window that contains the Squeak display.
The handle in this case would be a 32-bit or 64-bit C pointer, and the
expectation would be that the image should treat this as an opaque handle,
meaningful to the VM but serving only as a handle reference within the
image.

My opinion:

For our immediate purposes, we should respect the convention of treating
host window handle 1 as a reference to the display window. Hack the plugin
and X11 loadable VM module as needed make it work. It does work, and I'll
post some VM patches as soon as I can (maybe a couple of days, I'm done
looking at it for now).

But what we really should do: Implement it as I believe that it was probably
done at Qwaq. The handles should be opaque, and the host window plugin
should have an addition primitive to answer the window handle for the
main host window. That primitive would, in the case of the X11 VM module,
call this to get the handle:

  void *display_ioGetWindowHandle() { return (void *)stParent; }

This can be done in a backward compatible manner, because the image
can call the primitive to obtain the main window handle, and fall back
to a default value of 1 if the primitive fails.

Dave