Hi Dan,

  I know that you sent your SCA (repeatedly =) so
  I looked at the fix.

  It looks good.

  Please see my comments below.

Dan Munckton wrote:
APPROACH

The fix is really simple it just checks to make sure RANDR's version is
1.2 or greater if usingXinerama is true, if this is all fine it proceeds
to load the libXrandr funcs.

  Sounds good.

For the moment I've completely ignored 6599351, and not touched any of
the Xinerama loading code at all.

  OK, I think it's a good idea to separate the two fixes.

BTW I note that with 6599351 the user has an old style X dual-head
config without using Xinerama - I found a note on the Debian Xrandr1.2
Howto wiki page [1] explaining that this configuration should crash
Xserver 1.3. Does it behave differently in Solaris X?

  You mean, if randr is present?
  It could be that it didn't have randr extension.
  It looks from the comments in the bug report that 1.3 works
  fine with dual screens w/o xinerama in general.

TESTING

The equipment I have here will allow me to test single monitor setups
with X servers 1.2 and 1.3. Java now behaves as expected in the
following cases:

1) Xserver 1.2 + 1 monitor + Xrandr
        Expect: isFullScreenSupported: true
        Result: PASS

2) Xserver 1.2 + 1 monitor + Xinerama enabled (Xinerama won't actually
load but Xrandr and DRI won't load either)
        Expect: isFullScreenSupported: false
        Result: PASS

3) Xserver 1.3 + 1 monitor + Xrandr + fake Xinerama
        Expect: isFullScreenSupported: true
        Result: PASS

4) Xserver 1.3 + 1 monitor + Xinerama enabled (Xinerama won't actually
load but Xrandr and DRI won't load either)
        Expect: isFullScreenSupported: false
        Result: PASS

  I assume this is all on linux, right?

TESTING TODO

I have an external LCD monitor at work which I can hook up to my laptop
next week. This should allow me to test out Xrandr 1.2 multi-monitor
setups. I'll post results as soon as complete.

However I don't think I can test out Xinerama dual head configs. I tried
to set this up once before but failed - I'm still not 100% certain if
Xinerama is actually compiled into my X server I will need to check this
out properly.
If anyone here has a multi monitor setup already and would be prepared
to help me test the following scenarios I'd be very grateful.

5) Xserver 1.2 + 2 monitor + Xinerama (Xrandr and DRI won't load)
        Expect: isFullScreenSupported: false

6) Xserver 1.3 + 2 monitors + Xinerama
        Expect: isFullScreenSupported: false

  Ugh. That might be a problem - we don't have too many
  multiscreen solaris or linux systems that we could install
  new X server on. But we'll see what we can do.

  We can at least test on the configuration we have.
  I have a Solaris 10 machine with dual (non-xinerama)
  screen - the fs mode isn't supported on it.

I am also still to run the jtreg tests against this. I will come back
with results.

  I can't think of a good automated regression test for this fix.
  You can I suppose write a jtreg regression test as script
  (it is allowed), run xdpyinfo and find out that randr of the
  correct version is installed and then test if fullscreen
  is supported but it may not necessarily be a fully correct test -
  fullscreen may not be enabled for other reasons.

Also, do I need to mail both awt-dev and 2d-dev or will just one do in
future?

  This particular fix is 2D only, so we can continue on 2d-dev.
  The other one - we'll see. I suppose it's not that much of a problem
  if you cc both lists.

  Thanks,
    Dmitri



Cheers

Dan


[0] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6636469
[1] Section VI.3 of http://wiki.debian.org/XStrikeForce/HowToRandR12

Reply via email to