<AWT Dev> [9] Review Request: 6815345 java.awt.Component.createImage(int width, int height) should remove behavioral optionality

Semyon Sadetsky semyon.sadetsky at oracle.com
Fri Sep 25 18:57:22 UTC 2015 

Hi Phil,

Thank you for clarification.
JComponent's createImage() is called through the RepaintManager which 
does LW painting.
I worried about the server off-screen painting possibility that could be 
organized in a JDK port using peer based approach. But probably the 
original specs did not assume it.
So the fix looks good.

--Semyon


On 9/25/2015 8:51 PM, Phil Race wrote:
> There seems to be a lot of back and forth here.
>
> Fundamentally what this bug is about is changing one word: "May" -> "Will"
>
> So the question to be answered is can we make that change ?
>
> As I think was noted somewhere  in the thread, in headless mode,
> the heavyweight AWT components can't exist to begin with.
> Still,  I think it is perfectly fine and helpful for these methods to 
> indicate
> what they do in headless mode so long as it is in fact the case.
>
> And in headful mode, components which have not yet been associated
> with a display may not be able to allocate the resource, so we can
> consistently return null in that case.
>
> The interesting one is lightweight components where the association
> with a native display resource is a lot more distant. However I do
> not see JComponent over-riding createImage, so unless I am missing
> something that should be fine too.
>
> So with that caveat I think this change is fine.
>
> Some background notes about headless and peers :
>
> Headless mode basically means there is no possibility of connecting
> to a display server.
> So the only things that are available are those
> that can be executed without such a platform resource.
> This therefore disallows top-level windows and the set of AWT components
> which are considered 'heavyweight'.
>
> The details of how headless mode  is implemented depends on the platform.
> On Solaris it means we do not even link against X11. It does not even need
> the X11 libraries installed on the O/S
> On Windows we still link against GDI, we just disallow using it.
>
> So you can largely thing of headless mode as "no AWT mode"
> Java 2D in general does not need a display server.
> [Do not conflate package names containing java.awt with being AWT.
> There is no "java.2d" package so it is all mixed in.]
> Java 2D can draw quite happily to a system/java heap
> memory image and send that to any destination.
> Fonts and printing (part of 2D) do not need a display server.
>
> Also being able to access a peer is not something that applications
> can/should do. Peer is a concept that is discussed but an actual peer
> type is internal to the implementation. JDK 9 has updates that make
> this more clear than it was.
>
> -phil.
>
> On 09/25/2015 10:13 AM, Semyon Sadetsky wrote:
>>
>>
>> On 9/25/2015 7:06 PM, Sergey Bylokhov wrote:
>>> On 25.09.15 18:38, Semyon Sadetsky wrote:
>>>>
>>>>
>>>> On 9/25/2015 4:51 PM, Sergey Bylokhov wrote:
>>>>> On 25.09.15 15:44, Semyon Sadetsky wrote:
>>>>>>
>>>>>>
>>>>>> On 9/25/2015 2:46 PM, Sergey Bylokhov wrote:
>>>>>>> On 25.09.15 10:13, Semyon Sadetsky wrote:
>>>>>>>> Native container based components are disabled. But other native
>>>>>>>> resources can be used if available.
>>>>>>>
>>>>>>> no hw components -> no backbuffers for them.
>>>>>> In my understanding it depends on the component. If component 
>>>>>> capable to
>>>>>> work in the headless mode it can continue to use its peer, can't it?
>>>>>>>
>>>>>>>> All **Peer interfaces are public and external. Peer is not a 
>>>>>>>> hidden or
>>>>>>>> implementation specific term. It isn't allowed to application
>>>>>>>> developers
>>>>>>>> for implementation, but it is used for porting client libs to 
>>>>>>>> other
>>>>>>>> platforms. Java specification has wider vision than one particular
>>>>>>>> implementation. And for porting the headless mode is very 
>>>>>>>> sensitive
>>>>>>>> topic because specific platforms are headless often.
>>>>>>>
>>>>>>> The java.awt.peer package is private and should be used by the awt
>>>>>>> developers only. If the new port will be implemented then it should
>>>>>>> follow the same specification as our implementation.
>>>>>
>>>>>> That's correct. And our specification should be reasonable in its 
>>>>>> turn.
>>>>>> It must not be simply back-filled from our code otherwise it would
>>>>>> unlikely have more than one implementation.
>>>>>> JCK team likes direct specs in "if-then" style. But I see that the
>>>>>> initial doc was soft enough because it just clarify that 
>>>>>> exceptions from
>>>>>> the general rule are possible depending on the specific 
>>>>>> implementation
>>>>>> concept to leave more possibilities for porting.
>>>>>
>>>>> But it was not strict enough, we always return null for a long time,
>>>>> all other ports do the same because jck require this. So this is 
>>>>> not a
>>>>> problem for possible ports and this removes possible misleading
>>>>> assumption for application developers. This fix is a spec
>>>>> clarification that an application should not try to create a buffer
>>>>> for the hw component in such situations.
>>>>>
>>>>> The article I send you
>>>>>> also contains a lot of soft phrases.
>>>>>>>
>>>>>>>>>
>>>>>>>>> Maybe
>>>>>>>>>> Component#getImage() implementation should simply check for
>>>>>>>>>> isHeadless()
>>>>>>>>>> and return null?
>>>>>>>> But what will happen if Unix DISPLAY variable is not set so
>>>>>>>> isHeadlessInstance()=true?  Will peer be allowed? Should component
>>>>>>>> buffering work smoothly?
>>>>>>>
>>>>>>> The peers are not allowed in the headless mode. The DISPLAY var 
>>>>>>> is not
>>>>>>> specified in the javadoc, this means that some implementation 
>>>>>>> can work
>>>>>>> when DISPLAY is empty/unset, but in this case they must return 
>>>>>>> false
>>>>>>> from the isHeadlessXX();
>>>>>> isHeadlessInstance() just tests DISPLAY on Linux.
>>>>>
>>>>> This is our implementation, other can check other things. There are
>>>>> some implementation that can work without display, but in this case
>>>>> this environment is not headless.
>>>>>
>>>>> Can you specify what sentence you complain?
>>>>> This cannot be removed since it was there already
>>>>> "This will always happen if
>>>>> * <code>GraphicsEnvironment.isHeadless()</code> returns
>>>>> * <code>true</code>."
>>>>
>>>> "The return value may be null if the component is not displayable. 
>>>> This
>>>> will always happen if GraphicsEnvironment.isHeadless() returns true."
>>>>
>>>> your new version:
>>>>
>>>> "Returns null value if the component is not displayable or
>>>> GraphicsEnvironment.isHeadless() returns true"
>>>>
>>>> which invited me as a reviewer to find an "if" in the method body or
>>>> deeper, because it sounds "It shall return null if ..." .
>>>> But I didn't find it. Imagine if you get this spec to be implemented
>>>> from scratch. Why 'if' is not added as the first line? That would
>>>> eliminate all questions.
>>>
>>> Is this check in the code is not clear?
>>> (peer != null) ? peer.createImage(width, height) : null
>> It is not obvious. It should check 
>> if(GraphicsEnvironment.isHeadless()) {return null;}. Why to execute 
>> anything else?
>> Look on the similar spec and its implementation:
>>
>>      * @exception HeadlessException if GraphicsEnvironment.isHeadless()
>>      * returns true
>>      * @see java.awt.GraphicsEnvironment#isHeadless
>>      */
>>     public Button(String label) throws HeadlessException {
>>         GraphicsEnvironment.checkHeadless();
>>         this.label = label;
>>     }
>>>
>>>>
>>>> The previous version doesn't demand "if" because it is indirect and as
>>>> developer I would never rely on the null value promised by such spec.
>>>> But with the new version of the spec the null should be guaranteed for
>>>> the specified conditions otherwise we receive a bug.
>>>
>>> And if an application, which will tested on your code, will start on 
>>> our jdk it will fail with npe.
>>>
>> No, its reversed. I meant "relay on null value". That means any logic 
>> that expects null value only at this conditions may fail.
>>>>
>>>> And I cannot see the reason why non-null value cannot be allowed 
>>>> because
>>>> it doesn't brake anything.
>>>> Explanation "we define this and developers should follow" is a bit
>>>> frustrating because I see scenarios when component buffer could be
>>>> useful in the headless mode.
>>>
>>> This is not about a headless.
>> Why? I provided you reference to the article in which states 
>> opposite. Peer#createVolatileImage() may be used in headless.
>> For example, a network server without GUI can render images and send 
>> them to client via network or to printer. Why it cannot use its 
>> available hardware for image rendering?
>> Maybe there are some obstacles for that which I don't know. I'd 
>> appreciate it if you provide them.
>>>
>>>>
>>>> I suspect the original spec didn't imply strict binding to the values,
>>>> and this fix may simplify testing but distorts the original approach.
>>>
>>> It is always good to specify strict and clear null behavior to 
>>> suppress possible surprises.
>>>
>>>> I just unsure that this issue should be resolved. Maybe other 
>>>> reviewers
>>>> could add their opinion.
>>>>>
>>>>>
>>>>>>>
>>>>>>>> Note this is a common network server scenario which runs Java 
>>>>>>>> app from
>>>>>>>> the text console. I cannot get from the spec is it completely
>>>>>>>> different
>>>>>>>> headless mode or it is the same as the global headless?
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> And more global question: Why should we disable the creating of
>>>>>>>>>> component image buffer for the headless mode?It could be used 
>>>>>>>>>> for
>>>>>>>>>> the
>>>>>>>>>> same performance reason as in non-headless.
>>>>>>>>>
>>>>>>>>> in the headless mode we cannot create most of the hw components,
>>>>>>>>> actually non of top level window can be created. And for lw
>>>>>>>>> components
>>>>>>>>> like in swing the one global buffer is used in the 
>>>>>>>>> RepaintManager.
>>>>>>>> One global buffer means one global repaint
>>>>>>>
>>>>>>> No this is not true. If one component is changed then only one
>>>>>>> component is repainted.
>>>>>>>
>>>>>>> off all components for any
>>>>>>>> single component change. Why can't we be isomorphic here?
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> The headless case is covered, heavyweight components 
>>>>>>>>>>>>> cannot be
>>>>>>>>>>>>> created
>>>>>>>>>>>>> in such mode, so only lightweight buttons are checked.
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 24.09.15 15:58, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 9/24/2015 3:25 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>>>>> On 24.09.15 11:36, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>> Hi Sergey,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>   isHeadless()=true must return null. If so please add the
>>>>>>>>>>>>>>>> corresponding
>>>>>>>>>>>>>>>> test case. It is not not obvious from the code.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> isHeadless()=true is headless mode where the frames are 
>>>>>>>>>>>>>>> always
>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>> displayable, so everything is similar to the current test
>>>>>>>>>>>>>>> except
>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>> in headless the pack() will be throw an exception and 
>>>>>>>>>>>>>>> second
>>>>>>>>>>>>>>> part of
>>>>>>>>>>>>>>> the test in this mode is unnecessary.
>>>>>>>>>>>>>> Then maybe simply do not call pack() for the headless test?
>>>>>>>>>>>>>> Okay, let me rephrase what I meant. Since isHeadless()=true
>>>>>>>>>>>>>> case is
>>>>>>>>>>>>>> mentioned in those 3 specs so explicitly it must be 
>>>>>>>>>>>>>> guarantied
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> specified behavior works for the case as described. I cannot
>>>>>>>>>>>>>> trace
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>> result by reading the code of the createImage(), 
>>>>>>>>>>>>>> isHeadless()
>>>>>>>>>>>>>> method is
>>>>>>>>>>>>>> not even called there.  So the test case should be added. 
>>>>>>>>>>>>>> Or you
>>>>>>>>>>>>>> could
>>>>>>>>>>>>>> remove isHeadless() references from the specs. Or write
>>>>>>>>>>>>>> something
>>>>>>>>>>>>>> like
>>>>>>>>>>>>>> "the result is non-deterministic if isHeadless() is not
>>>>>>>>>>>>>> false..."
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 9/23/2015 9:14 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>>>>>>> Hello.
>>>>>>>>>>>>>>>>> Please review the fix for jdk9.
>>>>>>>>>>>>>>>>> The specification is updated as suggested in 
>>>>>>>>>>>>>>>>> JDK-6186530 and
>>>>>>>>>>>>>>>>> JDK-6815345. The test is added to prove that we always 
>>>>>>>>>>>>>>>>> return
>>>>>>>>>>>>>>>>> null
>>>>>>>>>>>>>>>>> when the component is not displayable.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> ccc request will be created after the technical 
>>>>>>>>>>>>>>>>> review. One
>>>>>>>>>>>>>>>>> additional
>>>>>>>>>>>>>>>>> bug filed 
>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8137047
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6815345
>>>>>>>>>>>>>>>>> Webrev can be found at:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~serb/6815345/webrev.04
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20150925/161e0843/attachment-0001.html>

More information about the awt-dev mailing list