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

[Oleg Sukhodolsky]

Hi Dmitri,

I understand than you do not want to add questionable code in 2D,
but I do not like the idea to make this by adding questionable code
to Swing. So before falliwing back to hack I'd suggest to thinkabout
possible API, otherwise we have a good chance to keep this hack
in Swing code forever :(

Oleg.

On Tue, Sep 9, 2008 at 10:16 PM, Dmitri Trembovetski
<Dmitri.Trembovetski at sun.com> wrote:
>
>  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
>>>
>