Re: [OpenJDK 2D-Dev] [9] RFR JDK-8170578: The Pages Range From in print dialog is disabled
I spent some significant amount of time trying to make sure I know the
IPP protocol
that is underpinning this. Prasanta provided me with some additional
logging from
which it seems that there are 3 repeated attributes.
:
IPPPrintService>> readIPPResponse pwg-raster-document-resolution-supported
IPPPrintService>> readIPPResponse pwg-raster-document-sheet-back
IPPPrintService>> readIPPResponse pwg-raster-document-type-supported
Furthermore I have determined that 16.10 ships with CUPS 2.2 - released
only a few weeks before 16.10
Whereas 16.04 has 2.1.3. So what we have here is a major CUPS upgrade
and I am still not 100% sure
if these duplicates are a CUPS bug - but I think it probable.
These duplicated attributes are not important to our implementation, so
all we need to do
is make sure they don't break us. I don't see a reason to care if we
keep the first or replace
it with the second. Any new attributes we've previously ignored will
still be ignored.
What Prasanta has implemented will fix our bug so I am approving.
However our implementation looks odd to me - I do not understand why there
was an expectation a new HashMap ever needed to be created. It only makes
sense if duplicate attributes are normal. But then there is no rhyme nor
reason
to then simply picking the first HashMap and ignoring the rest.
If anything I'd expect that you may want to tie a HashMap to each group tag.
In which case the first one would be operational attributes, not printer
attributes.
And it isn't done like that anyway. It just creates a new one "on
demand" when
it sees the key is already used. This happens in the middle of
processing a particular group.
So it still makes no sense.
Accordingly I think we need a follow-on bug to examine, explain and if
necessary update this in 10.
-phil.
On 1/18/17, 3:11 AM, Prasanta Sadhukhan wrote:
Ok. Updated webrev just in case we are not reading the protocol wrongly.
http://cr.openjdk.java.net/~psadhukhan/8170578/webrev.02/
Regards
Prasanta
On 1/12/2017 8:04 AM, Philip Race wrote:
I am looking at this and the hold up is I can't remember the protocol
readIPPResponse() is
trying to follow. Without that understanding it is hard to say much
.. so I need to go read
up and remember ..
Two comments : There is an extra space in
if (responseMap.length> 1) {
The test update is bogus in referring to this bug. The change is
nothing to do with the bug.
And I actually prefer printer tests to throw nasty exceptions when
there are no printers
and they need one ... else SQE just don't configure printers on test
systems and the tests all wrongly pass.
Would you run Linux UI tests on a blade with no X-server running ?
-phil.
On 1/10/17, 1:14 AM, Prasanta Sadhukhan wrote:
On 1/10/2017 2:43 PM, Prasanta Sadhukhan wrote:
Actually, in ubuntu16.10 attribute map did not have
"page-ranges-supported" attribute because 2 attribute hashmap is
created in IPPPrintService#readIPPResponse() and most of the
supported attributes are part of the 2nd hashmap (responseMap[1])
whereas only the 1st hashmap is utilised through responseMap[0]
[http://hg.openjdk.java.net/jdk9/client/jdk/file/8be0bb1aa238/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1760]
As to why 2 hashmaps are created in ubuntu16.10, it is because
readIPPResponse() checks if key is already present in existing map,
then create a new hashmap through this code
and in ubuntu16.10, some attributes (like
pwg-raster-document-type-supported which is not there in ubuntu
14.04] are duplicated.
Proposed fix is to check if there are more than 1 hashmaps, if it
is, get the entries from those maps, remove duplicate entries and
append to existing hashmap to get a consolidated map having all the
IPP attributes.
Missing webrev link:
http://cr.openjdk.java.net/~psadhukhan/8170578/webrev.01/
Regards
Prasanta
On 12/21/2016 11:28 PM, Phil Race wrote:
So now I am very suspicious.
First (previous fix I reviewed) Job Sheets weren't supported on
16.10, now its PageRanges.
I think this merits investigation of what is going on before we
commit this fix.
Why are attributes that have always been supported by CUPS
suddenly unavailable ?
And for the cases we image the pages ourselves, we can implement
PageRanges
internally, so for a normal PrinterJob it always can be supported
.. regardless of
what the printer says.
-phil.
On 12/14/2016 09:50 PM, Prasanta Sadhukhan wrote:
Hi All,
Please review a testbug fix for jdk9 for an issue where it it
seen that PageRanges option is disabled in printer dialog in
ubuntu16.10.
Bug: https://bugs.openjdk.java.net/browse/JDK-8170578
webrev: http://cr.openjdk.java.net/~psadhukhan/8170578/webrev.00/
Issue was, in ubuntu16.10 the attribute map [obtained here
Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: ColorModel subclasses are missing hashCode() or equals() or both methods
I don't think there is a spec anywhere that 2 objects created by the same constructor must evaluate as equals(). In
particular, 2 instances of Object created with the null constructor do not evaluate as equals(). So, with respect to
testing a spec, they are reading a promise into the base class there.
On the other hand, it makes sense that similar construction would be equals() for objects intended to be compared and
the CM subclasses have always tried to be comparable. But, since CM is abstract you can't really test its constructors
directly. With these fixes we should end up having all subclasses compare as equals() for similar construction so we
are still making good on the implied promise that they are testing here for all constructable objects.
Should we consider that CM might promise proper equals() tests for subclasses? For one, we can't really fully promise
that since subclasses may differ for reasons other than what CM can test, so its implementation of equals() can't be
authoritative in general. It is true that we used to provide a conditionally appropriate test for just the parts that
CM was responsible for, but we never documented that. In practice that potential promise doesn't matter for our
internal classes, it only matters for external classes that aren't being updated here.
With regards to Phil's question about whether these changes will affect application code - since you can't instantiate
CM directly, and since we deal with these conditions properly in all sub-classes this boils down to whether applications
use their own subclass of CM, and I don't have a lot of data for that. Phil?
...jim
On 1/30/17 10:37 PM, Jayathirth D V wrote:
Hi,
Regarding Jim's query of #2 JCK test case:
How do they accomplish this when the CM class is abstract? Do they create a
relatively empty subclass and instantiate that?
Yes they have simple subclass(MyColorModel) of ColorModel and they instantiate
that with same "bits" and try to compare.
MyColorModel cm = new MyColorModel(4);
ColorModel x = new MyColorModel(4)
if (!cm.equals(x)) {
return Status.failed("Should return true on two instances
instantiated by the same constructor");
}
Thanks,
Jay
-Original Message-
From: Jayathirth D V
Sent: Tuesday, January 31, 2017 11:53 AM
To: Philip Race; Jim Graham; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: ColorModel
subclasses are missing hashCode() or equals() or both methods
Hi Phil & Jim,
Thanks for your inputs.
I have verified with previous iteration of the changes and both #1 and #4 fail from the
time we have introduced the change in how we calculate "nBits" in ColorModel.
-nBits = bits.clone();
+/*
+ * We need significant bits value only for the length
+ * of number of components, so we truncate remaining part.
+ * It also helps in hashCode calculation since bits[] can contain
+ * different values after the length of number of components between
+ * two ColorModels.
+ */
+nBits = Arrays.copyOf(bits, numComponents);
#2 and #3 would fail from the time we have removed equals() and hashCode()
methods from ColorModel, to follow identity-as-equals verification.
Collectively all of them are failing right now as we have both the changes at
http://cr.openjdk.java.net/~jdv/7107905/webrev.15/ .
Thanks,
Jay
-Original Message-
From: Phil Race
Sent: Tuesday, January 31, 2017 5:27 AM
To: Jim Graham; Jayathirth D V; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: ColorModel
subclasses are missing hashCode() or equals() or both methods
Sounds like we should at least try to get the tests updated so they only test
what the spec. says.
Although it does indicate that there is at least a chance that application code
might also fail due to similar assumptions.
Does #1 not fail with the previous iteration of this change too ?
-phil.
On 01/30/2017 01:40 PM, Jim Graham wrote:
Hmmm. Sounds like the test cases were written based on bugs in the
implementation. I'm not sure what the best tactic is here for the
short term for getting this in, but many of these changes should
eventually be considered bugs in the tests. Is it acceptable to break
API tests like this at the last minute even if the tests are at fault?
Phil?
Notes on specific instances below...
On 1/30/17 2:22 AM, Jayathirth D V wrote:
Hi Phil,
1)api/java_awt/Image/ColorModel/index.html#Constructor: Failed.
test cases: 4; passed: 3; failed: 1; first test case failure:
ColorModel2001
This test fails because getComponentSize() returned an array with
length 3 but it expects the length to be 4. In the test case they
have bits per component array of length 4 like {8, 8, 8, 8}. But
in the test case wherever they are passing "has Alpha" as "false" we
omit
