<Swing Dev> [OpenJDK 2D-Dev] [PATCH] SwingUtilities2.isLocalDisplay()

[Dmitri Trembovetski]

   Hi Oleg, Roman,

   I myself don't think that this API belongs in public.

   It is used by the platform implementation for doing
   specific tasks and is of questionable value for
   general public. Also, it is too platform-specific
   and typically we refrain from adding platform-specific
   APIs to the Java platform.

   How do you think the developers would use it anyway?

   Those developers who know what a "remote display" is -
   which I assume is a minority since most developers never
   leave the Windows world - can detect it themselves if
   they really needed something like this - it's not a
   big deal, just read the DISPLAY env. variable and
   figure it out.

   In the specific case of SwingUtilities I believe they
   only use it to determine whether to enable AA text
   or not. So perhaps that's something that should
   be exposed instead.

   And talking about implementation - if we were to add
   something to this effect to the platform, we'd need
   to go all the way and possibly detect remote desktop
   session on Windows as well. And what about VNC sessions?
   In our current implementation we only care about
   "remote X display" situation, but the developers who
   see this public API could assume otherwise.

   I suggest to fall back to the original Roman's
   patch which just exposed this method in
   SunGraphicsEnvironment.

   Thanks,
     Dmitri

Oleg Sukhodolsky wrote:
> I'd vote for this API, but it is 2D team who should make the decision.
> 
> Oleg.
> 
> On Mon, Sep 8, 2008 at 11:11 PM, Roman Kennke <roman.kennke at aicas.com> wrote:
>> Bah, forgot the actual patch. Here it comes now!
>>
>> /Roman
>>
>> Am Montag, den 08.09.2008, 21:04 +0200 schrieb Roman Kennke:
>>> Hi Oleg and lists,
>>>
>>>>>> I'm not the person who is supposed to review this changes from
>>>>>> technical point of view,
>>>>>> but I have one concern about idea of the fix.  It adds one more place
>>>>>> where Swing uses sun-specific API.
>>>>> No. It takes advantage of Sun internal API, but it does not depend on
>>>>> it. (if (ge instanceof SunGraphicsEnvironment)  { doSomethingCool(); }
>>>>> else { useReasonableDefault(); } ).
>>>> it uses Sun-specific class, so this code can not be compiled by JDK
>>>> from another vendor :(
>>> Ehe :-) Try compiling Swing on any JDK from another vendor! Good luck!
>>> (Ok, the Caciocavallo project will certainly help alot, but we are still
>>> very far away from such a portable Swing).
>>>
>>>>>> Of course you can say that SwingUtilities2.isLocalDisplay() even
>>>>>> before your fix uses code which is specific
>>>>>> for Sun's implementation, and you will be right.  But this doesn't
>>>>>> mean that we should leave with this.
>>>>>> IMHO, if we change such code we should remove dependency between Swing
>>>>>> and Sun-specific code,
>>>>>> i.e. we should add public API.
>>>>> I completely agree. That, of course, would be the best solution. But in
>>>>> the past it has been communicated several times to me that I can't just
>>>>> send in patches to public API and hope that it will be accepted easily.
>>>>> I propose to get in this patch if possible first, then start a
>>>>> discussion about adding public API.
>>>> I understand your point, but I have a feeling that we you will not
>>>> start the discussion right now,
>>>> the idea of new API may be lost :(  So, I'd suggest to try one more time ;)
>>> Ok, so here we go. This patch is slightly modified, the isDisplayLocal()
>>> method is now declared in GraphicsEnvironment, with the addition of
>>> throwing a HeadlessException in the headless case. Also, in fontpath.c,
>>> we don't call isDisplayLocal(), but instead call _isDisplayLocal(). We
>>> call it on X11GraphicsEnvironment only. In my original patch I tried to
>>> be more portable by calling
>>> GraphicsEnvironment.getLocalGraphicsEnvironment().isDisplayLocal(), but
>>> this resulted in an initialization loop (this code is very sensible to
>>> this kind of problem). I left that out for now, because fontpath.c is
>>> X11 specific anyway, and a real solution will come soon in the form of a
>>> huge FontManager patch :-)
>>>
>>> What do you think? Can we add this API to GE?
>>>
>>> /Roman
>>>
>> --
>> Dipl.-Inform. (FH) Roman Kennke, Software Engineer, http://kennke.org
>> aicas Allerton Interworks Computer Automated Systems GmbH
>> Haid-und-Neu-Straße 18 * D-76131 Karlsruhe * Germany
>> http://www.aicas.com   * Tel: +49-721-663 968-48
>> USt-Id: DE216375633, Handelsregister HRB 109481, AG Karlsruhe
>> Geschäftsführer: Dr. James J. Hunt
>>