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
