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

[Dmitri Trembovetski]

   Hi Oleg,

Oleg Sukhodolsky wrote:
> 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

   I'm not sure what you mean by "adding questionable code
   to Swing" - the code was already there. Now instead of
   directly calling sun internal API it will call a better
   defined internal API.

> possible API, otherwise we have a good chance to keep this hack
> in Swing code forever :(

   Swing is part of the platform, and will always have to
   use some APIs in the implementation which are not available
   to external developers.

   I don't see a problem with that. In this particular case
   I really don't see any benefit in exposing this very
   platform-specific API to the developers.

   Thanks,
     Dmitri


> 
> 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
>>>>