[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-7116979 : Unexpected pixel colour when converting images
From: Jim Graham <james.graham () oracle ! com>
Date: 2016-02-19 21:32:20
Message-ID: 56C789E4.6000409 () oracle ! com
[Download RAW message or body]
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
> Average time 871167ns
>
> After change :
> Minimum time 536380ns
> Maximum time 1468130ns
> Average time 830778ns
>
> There is +/- 10% delta both for before and after change time but there is no major \
> gap. Even for image with other input type it holds the same.
>
> Regarding custom color maps, previously I ran on default color map generated in \
> TYPE_BYTE_INDEXED constructor. Now I tested with modified color map so that it \
> doesn't contain any of the primary color values. Instead of using 0-255 values with \
> value 51 increments in R,G,B components I used color map with 20-245 values with \
> value 45 as increment. For this color map before and after change all the pixels \
> are referring to same index in color map irrespective of removal of error delta \
> addition in case of primary colors.
> I have attached log for Red and Green primary colors before and after change(Was \
> not able to attach for more since it exceeded 120KB mailing list limit). Please \
> search for "Final index value =" in the logs. Please let me know your inputs.
> Thanks,
> Jay
>
> -----Original Message-----
> From: Jim Graham
> Sent: Tuesday, February 16, 2016 1:03 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,
>
> The issue with testing performance with white images is that the tests allow the \
> code to eliminate 3 indexed fetches which take time. Primary colors end up being \
> the one obscure case where the new code might be faster. But, non-primary colors \
> would be slower by a fair amount, so performance testing with converting a \
> randomized or non-primary-color image are the more telling case. An image filled \
> with 255,255,128 would be the worst case because it would involve all of the tests \
> and still have to do all of the lookups.
> My question about how this works with custom colormaps was not really addressed by \
> your response. A simple test to make sure the colormap has accurate (or very \
> close) conversions for the primary colors may be enough to validate the colormap \
> for the indicated tests. If the colormap contains no pure conversions for some of \
> the primary colors then they may need to be dithered anyway...
> ...jim
>
> On 2/15/16 3:39 AM, Jayathirth D V wrote:
> > Hi Jim,
> >
> > I performed performance analysis with white image so that all
> > conditions are checked in the new branch added. There is no major
> > difference in time taken before and after change. For each input I
> > have tested time taken for drawImage() API to convert from every
> > format to Byte indexed type. For every unique conversion test is run
> > for 100 times. Please find the details:
> >
> > Input
> >
> > type
> >
> >
> >
> > Min
> >
> > before change
> >
> > (ns)
> >
> >
> >
> > Min
> >
> > after change
> >
> > (ns)
> >
> >
> >
> > Max
> >
> > before change
> >
> > (ns)
> >
> >
> >
> > Max
> >
> > after change
> >
> > (ns)
> >
> >
> >
> > Average
> >
> > before change
> >
> > (ns)
> >
> >
> >
> > Average
> >
> > after
> >
> > change
> >
> > (ns)
> >
> > INT_RGB
> >
> >
> >
> > 523437
> >
> >
> >
> > 481491
> >
> >
> >
> > 1230724
> >
> >
> >
> > 1270440
> >
> >
> >
> > 789452
> >
> >
> >
> > 666144
> >
> > INT_ARGB
> >
> >
> >
> > 500232
> >
> >
> >
> > 493986
> >
> >
> >
> > 12406307
> >
> >
> >
> > 1308816
> >
> >
> >
> > 793384
> >
> >
> >
> > 654015
> >
> > INT_ARGB_PRE
> >
> >
> >
> > 500233
> >
> >
> >
> > 492201
> >
> >
> >
> > 1037057
> >
> >
> >
> > 981277
> >
> >
> >
> > 710250
> >
> >
> >
> > 699214
> >
> > INT_BGR
> >
> >
> >
> > 537716
> >
> >
> >
> > 562706
> >
> >
> >
> > 1446703
> >
> >
> >
> > 2046001
> >
> >
> >
> > 862377
> >
> >
> >
> > 863256
> >
> > 3BYTE_BGR
> >
> >
> >
> > 483275
> >
> >
> >
> > 481045
> >
> >
> >
> > 1181638
> >
> >
> >
> > 1384676
> >
> >
> >
> > 651427
> >
> >
> >
> > 580961
> >
> > 4BYTE_ABGR
> >
> >
> >
> > 544410
> >
> >
> >
> > 499786
> >
> >
> >
> > 1292305
> >
> >
> >
> > 968783
> >
> >
> >
> > 690106
> >
> >
> >
> > 683881
> >
> > 4BYTE_ABGR_PRE
> >
> >
> >
> > 504249
> >
> >
> >
> > 505588
> >
> >
> >
> > 1680086
> >
> >
> >
> > 1216445
> >
> >
> >
> > 756101
> >
> >
> >
> > 687750
> >
> > USHORT_565_RGB
> >
> >
> >
> > 507818
> >
> >
> >
> > 505588
> >
> >
> >
> > 978153
> >
> >
> >
> > 1346746
> >
> >
> >
> > 652908
> >
> >
> >
> > 655782
> >
> > USHORT_555_RGB
> >
> >
> >
> > 510496
> >
> >
> >
> > 509604
> >
> >
> >
> > 952272
> >
> >
> >
> > 1162004
> >
> >
> >
> > 650418
> >
> >
> >
> > 670811
> >
> > BYTE_GRAY
> >
> >
> >
> > 481491
> >
> >
> >
> > 478367
> >
> >
> >
> > 1140585
> >
> >
> >
> > 1799231
> >
> >
> >
> > 691160
> >
> >
> >
> > 583250
> >
> > USHORT_GRAY
> >
> >
> >
> > 506927
> >
> >
> >
> > 507373
> >
> >
> >
> > 1375751
> >
> >
> >
> > 1255267
> >
> >
> >
> > 728202
> >
> >
> >
> > 646902
> >
> > BYTE_BINARY
> >
> >
> >
> > 541733
> >
> >
> >
> > 496217
> >
> >
> >
> > 1083466
> >
> >
> >
> > 959411
> >
> >
> >
> > 730527
> >
> >
> >
> > 728461
> >
> > The changes are tested with plain images having primary colors like
> > RED, GREEN, BLUE, BLACK and WHITE.
> >
> > Thanks,
> >
> > Jay
> >
> > -----Original Message-----
> > From: Jim Graham
> > Sent: Friday, February 12, 2016 4:05 AM
> > To: Jayathirth D V; 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,
> >
> > Did you do any performance analysis of this change? You are adding 6
> > tests and multiple branches to per-pixel code.
> >
> > The effectiveness of this technique depends on the colormap that we
> > have set up. For the BufferedImage.TYPE_INDEXED constructor we
> > produce a fairly nice colormap, but if someone creates a custom
> > colormap then the colors could be anything. We create a decent
> > inversion for just about any colormap, but that doesn't mean that
> > using only "the best match for solid red" will produce a better result
> > for a dithered approximation for red. It is true that if there is a
> > really good match for red then we should just use that, but if there
> > are no direct matches for red then we may choose to paint a red region
> > with solid orange even though there is another color in the colormap
> > that when mixed with orange approximates a red tone better. For
> > example, if a colormap contains no pure red, but
> >
> > contains:
> >
> > 240, 20, 0
> >
> > 240, 0, 20
> >
> > (I'm not sure if 20 is within our current error deltas that we use for
> > dithering, but this is an example not a test case.)
> >
> > Then using one of these alone might skew the color towards orange or
> > purple. Using both together in a dither pattern might keep the
> > overall hue impression as red, but with a small amount of noise in its \
> > saturation.
> > What types of colormaps was this tested with?
> >
> > ...jim
> >
> > On 2/11/16 6:37 AM, Jayathirth D V wrote:
> >
> > > Hi,
> >
> > >
> >
> > > _Please review the following fix in JDK9:_
> >
> > >
> >
> > > __
> >
> > >
> >
> > > Bug :https://bugs.openjdk.java.net/browse/JDK-7116979
> >
> > >
> >
> > > Webrev :http://cr.openjdk.java.net/~jdv/7116979/webrev.00/
> >
> > >
> >
> > > Issue : When Image containing black pixels are converted from any
> >
> > > format to Byte Indexed format some of the pixels are not black. They
> >
> > > are following pattern similar to dithering.
> >
> > >
> >
> > > Root cause : When we convert any format type to ByteIndexed we are
> >
> > > adding Error delta values to R,G,B components using dithering indices.
> >
> > > This is causing some pixels values to not point to proper index in
> >
> > > color table.
> >
> > >
> >
> > > Solution : There is no need to add error delta for primary colors
> >
> > > containing basic values in R,G,B components. Exclude such pixels from
> >
> > > delta addition.
> >
> > >
> >
> > > Thanks,
> >
> > >
> >
> > > Jay
> >
> > >
> >
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic