Re: [OpenJDK 2D-Dev] [9] RFR: JDK-6529030, , Java Printing: Print range > Selection gets enabled
Hi Jennifer,
If I do not set SunPageSelection attribute in setNativeAttributes() like
this
+ if ((flags & PD_NOSELECTION) != PD_NOSELECTION) {
if ((flags & PD_PAGENUMS) != 0) {
attributes.add(SunPageSelection.RANGE);
} else if ((flags & PD_SELECTION) != 0) {
attributes.add(SunPageSelection.SELECTION);
} else {
attributes.add(SunPageSelection.ALL);
}
+ }
without doing my fix in awt_PrintControl.cpp#InitPrintDialog() it will
disable the selection radio button for both invocation as
getSelectAttrib() returns PD_NOSELECTION.
I think my fix in InitPrintDialog()
http://cr.openjdk.java.net/~psadhukhan/6529030/webrev.01/
is required for this fix.
Regards
Prasanta
On 4/20/2016 3:52 AM, Jennifer Godinez wrote:
Hi Prasanta,
It looks to me that we missed to check (flags & PD_NOSELECTION) in
setNativeAttributes that's why we are setting SunPageSelection
attribute when we shouldn't. I think that is where we should put the
fix.
Jennifer
On 04/15/2016 03:34 AM, prasanta sadhukhan wrote:
Hi Phil,
On 4/13/2016 10:40 PM, Philip Race wrote:
This seems reasonable to me - we don't want "disable" the selection
radio button
and prevent the user from selecting it since our API does support it
as an option.
The way I first read the bug report was that it implied that the
first invocation
when the selection button was disabled was "right" and the 2nd
invocation
was "wrong" whereas it seems the other way around.
If the updated code (and my understanding) is correct then should we
not in fact be updating getSelectAttrib() so that it never returns
PD_NOSELECTION,
rather than "fixing it up" in this code ?
Of course you then need to look to see how we interpret & use a
return value of PD_NOSELECTIONfrom getSelectAttrib() on OS X and Linux.
getSelectAttrib() is not called in linux.
But it's called in osx and we determine to/from pages to determine
which radio button (All/Pages) to select
http://hg.openjdk.java.net/jdk9/client/jdk/file/735a130dc8db/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m#l388
Regards
Prasanta
I would really like to get Jennifer's input on this.
-phil.
On 4/13/16, 4:17 AM, prasanta sadhukhan wrote:
Hi Phil,
On 4/13/2016 4:52 AM, Phil Race wrote:
Hi,
My reading here :
https://msdn.microsoft.com/en-us/library/windows/desktop/ms646843%28v=vs.85%29.aspx
of the meaning of PD_NOSELECTION is that it disables the SELECTION
radio button such
that the user *cannot* select it.
Is that true ?
Yes, PD_NOSELECTION will disable SELECTION radio button.
If so this seems like it cannot be the right fix for this issue
and I am not sure why getSelectAttrib() would want to return that.
protected final int getSelectAttrib() {
if (attributes != null) {
SunPageSelection pages =
(SunPageSelection)attributes.get(SunPageSelection.class);
if (pages == SunPageSelection.RANGE) {
return PD_PAGENUMS;
} else if (pages == SunPageSelection.SELECTION) {
return PD_SELECTION;
} else if (pages == SunPageSelection.ALL) {
return PD_ALLPAGES;
}
}
return PD_NOSELECTION;
}
so if SunPageSelection is not added to attribute which was the case
in 1st instance so PD_NOSELECTION is returned
and we have in awt_PrintControl.cpp#InitPrintDialog()
if (selectType != 0) { selectType will be 4 for PD_NOSELECTION so
pd.Flags will be set and Selection radio button is disabled in 1st
instance
pd.Flags |= selectType;
}
Perhaps we never in practice return PD_NOSELECTION ?
I am also unsure what it means to set flags to PD_NOSELECTION |
PD_SELECTION
as the windows docs don't tell me enough.
It will still disable SELECTION radio button.
Maybe what we want is just that we do not cause PD_SELECTION to be
set
rather than setting PD_NOSELECTION.
I have modified the webrev to not set PD_NOSELECTION if
getSelectAttrib() returns NOSELECTION so it will mean Selection
radiobutton will be enabled now but will not be selected UNLESS
user selects
job.setDefaultSelection(JobAttributes.DefaultSelectionType.SELECTION)
explicitly in the application.
http://cr.openjdk.java.net/~psadhukhan/6529030/webrev.01/
Also, I added @requires (os.family == windows) tag to make sure the
test is run on windows only as in linux, osx we do not get the
"selection" option [only All and Page range] for this test.
Regards
Prasanta
But to decide what to do I need to know the real effect of
PD_NOSELECTION.
BTW about the test: you should make sure that the instructions are
valid on OS X and Linux ..
-phil.
On 04/06/2016 03:09 AM, prasanta sadhukhan wrote:
Hi All,
Please review a fix for jdk9.
Bug: https://bugs.openjdk.java.net/browse/JDK-6529030
webrev: http://cr.openjdk.java.net/~psadhukhan/6529030/webrev.00/
The issue was when using java.awt.print.PrinterJob instance mor
Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images to TYPE_BYTE_INDEXED
Hi Jim, As discussed we will not add dithering error values to primary colors with color map which represents Primary colors approximately(+/-5 delta). I have made changes based on this suggestion and added new function to calculate whether color map represents primary colors approximately or not. If it represents only then we will avoid adding dithering error values in case ByteIndexed destination. I have observed that in case of white color we are picking (252,252,252) or (246,246,246) and not (255,255,255) from colormap. What I have observed that out of 256 places we are storing RGB colors at start and then gray values. So we are picking final gray value and not in between white value. That's why I am not verifying white color validity in test case. Please find updated webrev for review : http://cr.openjdk.java.net/~jdv/7116979/webrev.01/ Thanks, Jay -Original Message- From: Jim Graham Sent: Saturday, February 20, 2016 3:02 AM To: Jayathirth D V Cc: 2d-dev@openjdk.java.net; Philip Race; Prasanta Sadhukhan Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images to TYPE_BYTE_INDEXED Hi Jayathirth, Thank you for attaching the detailed logs, but I don't have a program that can deal with the 7z files. Can you summarize what was learned from them? Also, the colormap you created, while subset, was a perfectly scaled cube. I was referring more to a random colormap where you might have 2 colors close to a primary, but neither is on a line from the primary to the 0.5 gray value so each has a different hue. That's something that can only be evaluated visually. One example of randomized colormaps is when we display via X11 onto an 8-bit display. That probably never happens any more, but we may not be able to allocate colors in the colormap in such a case and if another program has already allocated most of the dynamic colormap then we would not be able to add our own colors. This could be dealt with by simply marking our standard colormap as having primaries, or by scanning any given colormap and marking it if it has primaries, and then only perform this code on colormaps with the primaries. Solving it for our own colormap (or solving it for any colormap that has all primaries) would likely solve it for 99% of the vanishingly small number of developers who might run into this. But, in the end, what would we accomplish by adding this one very targeted fix? I'm also curious where the push for this is coming from. While we aren't perfect here, so much of the world today is focused on lossless image formats that I don't imagine there is much need for really good 8-bit indexed image support any more...? If anything, spatial dithering is considered a very low quality algorithm and error propagation dithering would handle all of the colormaps much better. We used to do error propagation dithering back in the JDK 1.1 days, but we dropped it when we introduced 2D because we added a bunch of ways to render small pieces of an image and only the current spatial dithering algorithm can be started in the middle of an operation. That doesn't mean that you can't still do error propagation since many operations already operate on the full image any way and you can still handle subset image operations by either converting the full image first and then copying or by tracing the errors from the edge of the image to the point of the subimage operation. We never bothered to upgrade our algorithms it just never seemed to be work that made sense to prioritize given that screens were overwhelmingly moving to full-color at the time of "Java 2" (and now to HDR in 2016+). 8-bit indexed isn't used much any more. If we want to make 8-bit destination images work better we'd be better off adopting error propagation dithering again rather than trying to tune this algorithm for black. It looks like the bug was closed briefly in 2014 and then reopened soon thereafter with no justification. It is true that we still have this anomaly, but it isn't clear why this is high enough priority to work on in 2016 when other bit depths would work better for destination imagery and other dithering algorithms would work better for most customers and even this targeted fix isn't perfect... ...jim On 2/16/2016 8:32 AM, Jayathirth D V wrote: > Hi Jim, > > Thanks for letting me know about the importance of custom color maps. I was > not able to understand what you mentioned previously since I am new to this > terminology. > > Regarding performance I checked with image having complete 255,255,128 pixels > and ran it for 100 times. I am running on Windows 7 Professional SP1 with > Intel i5-5300U CPU. Overall I don't see much difference in time taken for > drawImage() API before and after change. Details for INT_RGB image as input: > > Before change : > Minimum time 536827ns > Maximum time 1560947ns > Ave
[OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method
Hi, Please review the following fix in JDK9: Bug : https://bugs.openjdk.java.net/browse/JDK-8153943 This is subtask of https://bugs.openjdk.java.net/browse/JDK-6588409 Webrev : http://cr.openjdk.java.net/~jdv/8153943/webrev.00/ Issue : Some of the java.awt.image classes are missing either equals() or hashCode() method. Solution : Add missing equals() or hashCode() methods. Thanks, Jay
[OpenJDK 2D-Dev] [PATCH] JDK-8152680: Regression in GlyphVector.getGlyphCharIndex behaviour
Hello, I'd like to propose a patch for JDK-8152680 - an issue I've raised via bugreport.java.com earlier, hope someone can sponsor it. I have a Contributor status via agreement signed by Jetbrains. The issue is related to the extraction of glyph-to-character mapping from results of text layout, performed by Harfbuzz, when layout is requested for a specified substring of text string. For LTR case, existing code assumes that cluster values (which are later interpreted as character indices) are assigned by Harfbuzz starting from the beginning of substring, but actually it's done starting from the beginning of whole string (as mentioned by existing comment in HBShaper.c). For reference, this logic can be found at hb-buffer.cc:1470 (function hb_buffer_add_utf). The proposed fix is to take into account this numbering scheme by shifting cluster value correspondingly. GlyphCharIndicesTest test case is included to cover this fix. RTL case is not affected by the problem mentioned above, but there's another issue with it - cluster value generated by Harfbuzz is ignored completely, instead character index is taken to be equal to glyph index (in visual order). This will not work, e.g. in case when there are more glyphs than characters - some glyphs will reference non-existing characters. The proposed fix is just to use the same shifted cluster value, as for LTR case - I believe it works just as well in RTL case. GlyphCharIndicesRtlTest test case is included to cover RTL case, but I'm not sure it should be definitely committed, as it depends on a specific Windows font, which doesn't seem to be available by default in Windows 10 (even though it must be present in Windows Vista, 7 and 8). I couldn't find a better font on Windows, demonstrating the issue. Webrev for the fix is available at http://cr.openjdk.java.net/~avu/DmitryBatrak/JDK-8152680 (kindly posted by my colleague, having access to cr.openjdk.java.net). Best regards, Dmitry Batrak
Re: [OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method
Hi,
You removed the following test in CCM.java :
2941 if (obj.getClass() != getClass()) {
2942 return false;
2943 }
What this means is that before your change an instance of a subclass of CCM
would never be equals() to any direct instantiatation of CCM but after
your change it can be. I suspect the condition was there on purpose.
-phil.
On 04/20/2016 05:45 AM, Jayathirth D V wrote:
Hi,
_Please review the following fix in JDK9:_
Bug : https://bugs.openjdk.java.net/browse/JDK-8153943
This is subtask of https://bugs.openjdk.java.net/browse/JDK-6588409
Webrev : http://cr.openjdk.java.net/~jdv/8153943/webrev.00/
Re: [OpenJDK 2D-Dev] Fix for JDK-8074829 : Resolve disabled warnings for libawt_headless
2d-dev added. I am not sure but why "declaration in the code" is a bad thing and we should fix it? - DISABLED_WARNINGS_solstudio := E_DECLARATION_IN_CODE I cannot find the documentation in solaris studio for this warning. On 20.04.16 11:57, Ajit Ghaisas wrote: Hi, Bug : https://bugs.openjdk.java.net/browse/JDK-8074829 This bug is to remove warning suppressions from makefile and fix the warnings for libawt_headless library. I have removed following warning suppressions & fixed the warnings for libawt_headless library. DISABLED_WARNINGS_gcc := maybe-uninitialized int-to-pointer-cast DISABLED_WARNINGS_solstudio := E_DECLARATION_IN_CODE Warning suppression that cannot be removed : DISABLED_WARNINGS_solstudio := E_EMPTY_TRANSLATION_UNIT This is due to the fact that - jdk/src/java.desktop/share/native/common/java2d/opengl/OGLBlitLoops.c file becomes empty file in case of HEADLESS mode compilation. Request you to review following webrev : http://cr.openjdk.java.net/~aghaisas/8074829/webrev.00/ Regards, Ajit -- Best regards, Sergey.
Re: [OpenJDK 2D-Dev] Fix for JDK-8074829 : Resolve disabled warnings for libawt_headless
On 04/20/2016 12:27 PM, Sergey Bylokhov wrote: 2d-dev added. In fact all these are 2D. No AWT warnings here. I am not sure but why "declaration in the code" is a bad thing and we should fix it? - DISABLED_WARNINGS_solstudio := E_DECLARATION_IN_CODE I cannot find the documentation in solaris studio for this warning. I don't mind fixing it if it is still an issue but does the current compiler actually complain about it ? The SS11 -> SS12 upgrade might have got a more modern C compiler .. On 20.04.16 11:57, Ajit Ghaisas wrote: Hi, Bug : https://bugs.openjdk.java.net/browse/JDK-8074829 This bug is to remove warning suppressions from makefile and fix the warnings for libawt_headless library. I have removed following warning suppressions & fixed the warnings for libawt_headless library. DISABLED_WARNINGS_gcc := maybe-uninitialized int-to-pointer-cast What made that one go away ?? DISABLED_WARNINGS_solstudio := E_DECLARATION_IN_CODE Warning suppression that cannot be removed : DISABLED_WARNINGS_solstudio := E_EMPTY_TRANSLATION_UNIT This is due to the fact that - jdk/src/java.desktop/share/native/common/java2d/opengl/OGLBlitLoops.c file becomes empty file in case of HEADLESS mode compilation. Sigh .. there ought to be "informational" warnings as well as "risky practice" warnings and this should be in the former category. You could move something like the jni.h and jlong.h imports outside to see if that shuts it up. Not saying that is what we want to do but it would be interesting to check. -phil. Request you to review following webrev : http://cr.openjdk.java.net/~aghaisas/8074829/webrev.00/ Regards, Ajit
Re: [OpenJDK 2D-Dev] [9] Review request for 8152309 Seamless way of using image filters with multi-resolution images
On 20/04/16 00:08, Phil Race wrote:
I've gone up and down the API (and most of the implementation)
ResolutionVariantItem is really only used in the public API in one
place :-
ImageProducer defines :-
+default List>
getResolutionVariantItems() {
+return Arrays.asList(new ResolutionVariantItem<>(this, 1, 1));
The sort-of second place is that FilteredImageSource over-rides this.
The rest is the internal uses of the class (and its definition of course)
So do we need such a generic sounding API class for this one case, (or
are there other API-level uses?) and is there a different approach
which avoid the new class entirely ?
The ResolutionVariantItem class in this case is used to provide scale
factors for the ImageFilter and MediaTracker.
If we want to avoid this class it is necessary to understand from
which place these scale factors can be obtained.
The MultiResolutionImage has just getResolutionVariant(width, height)
method which allows to provide an image either from a set of resolution
variants or just generating an image with requested size.
It seems that generation an image for requested size is not an option
for multi-resolution Toolkit image just because it can be loaded by
MediaTracker.
MediaTracker.addImage(Image, id, width, height) method needs to load
all resolution variants from the provided multi-resolution image. First
it means that there should be finite number of them. The second is that
for passed width and height argument it needs to calculate size for the
resolution variants which are not loaded yet.
This is the first place where the scale factors need to be provided
with resolution variant:
MediaTracker.addImage(mriImage, id, w, h) ->
MediaTracker.addImage(resolutionVariant, id, w * rvScaleX, h * rvScaleY)
The second place where the resolution variant scales are used is
SunGraphics2D.drawImage() method when it draws a multi-resoltion image
with an applied filter.
Image mrImage = // create a multi-resolution image
ImageProducer filteredImageSource = new
FilteredImageSource(mrImage.getSource(), filter);
Image filteredImage = toolkit.createImage(filteredImageSource);
SunGraphics2D.drawImage() calls getResolutionVariant(mrImage, ...)
method to get a resolution variant with the requested destination width
and height to draw it. The resolution variant in this case is just a
ToolkitImage which source is FilteredImageSource an filter is a scaled
filter:
toolkit.createImage(new
FilteredImageSource(resulutionVariant.getSource(),
filter.getScaledFilterInstance(scaleX, scaleY)))
And the question is where the the scale factors can be obtained for
this case?
If there are others way to provide scale factors for the described
cases it will allow to not use new classes like ResolutionVariantItem.
Thanks,
Alexandr.
Assuming ResolutionVariantItem is the right name - and design - it
seems like it could use a bit more class javadoc, even if it is basically
a bag to hold a few things.
There are few formatting/typo things I noticed but no point in talking
about
those until the rest is near conclusion.
-phil.
On 04/05/2016 07:30 AM, Alexander Scherbatiy wrote:
Hello,
Could you review the fix:
bug: https://bugs.openjdk.java.net/browse/JDK-8152309
webrev: http://cr.openjdk.java.net/~alexsch/8152309/webrev.00
The purpose of the fix is to allow to apply an image filter for a
multi-resolution image to get new multi-resolution image so the code
below works without changes:
--
Image mrImage = getMultiResolutionImage();
ImageProducer mriProducer = new
FilteredImageSource(mrImage.getSource(), new CropImageFilter(x, y, w,
h));
Image filteredMRImage =
Toolkit.getDefaultToolkit().createImage(mriProducer);
--
The Image producer needs to be updated to contain a set of
resolution-variant image producers. It can be done introducing a new
MultiResolution[Image]Producer interface. However, the
FilteredImageSource which takes an original image producer as a
constructor argument needs to be declared as
MultiResolution[Image]Producer even for ordinary image producers.
I chose to add the getRVProducers() method directly to the
ImageProducer interface.
The option to add a method which request a resolution variant
producer for the given image size (getRVProducer(width, height) )
seems is not possible because the result multi-resolution image can
be loaded by a MediaTracker which can load only finite number of
resolution variants.
Applying an image filter to resolution-variant producers requires to
scale filters which use fixed image size (like CropImageFilter and
ReplicateScaleFilter).
There should be a way to get a scaled filter using the original one.
The resolution variant image producer need to provide necessary scale
factors for the used filter.
To do that getScaledFilterInstance(scaleX, scaleY) method is added to
the Image
