OK, thanks -- I'll add a `colorModel.getColorSpace().isCS_sRGB()` check here as well.
Take care, Daniel On Thu, Sep 4, 2025 at 11:55 PM Philip Race <philip.r...@oracle.com> wrote: > It doesn't seem like a change that needs a project. > I would make sure that call to colorModel.getRGB(..) isn't needed and I > think that you aren't doing that. > Basically it needs to be sRGB already (as that is the target color space). > > -phil. > > On 9/4/25 1:37 PM, Daniel Gredler wrote: > > Hi Jeremy, Laurent, > > Thanks for having a look. What sorts of changes are you thinking of when > you propose a separate project? > > For myself, I don't really have a bigger-picture vision of systematic > changes that I'd like to make. Similar to JDK-8337681, JDK-8344668 and > JDK-8356814, this suggestion is just an ad hoc optimization opportunity > that came up based on spending some time with a profiler. > > My takeaway right now is that this specific suggestion doesn't look > obviously stupid to anyone, and is worth triple checking on my end. If my > local sanity checks all come back green, I would create a small issue in > JBS and submit a PR, and then let the review process carry on as usual. It > can take a little time, but it seems to me that useful optimizations, > submitted individually, can be incorporated without too many issues. > > Which takes me back to my original question -- what type of change(s) are > you thinking about, where proving things out in a separate project is > necessary? > > Take care, > > Daniel > > > On Sat, Aug 30, 2025 at 11:47 AM Laurent Bourgès <bourges.laur...@pm.me> > wrote: > >> Hi guys, >> >> I do love optimizing java2d, so Ido support & sponsor your buffered image >> (ARGB PRE or not) changes. >> >> Let's start a github project to start this concrete patch... as I did for >> the marlin renderer or we could use the marlin repository to host your >> changes to java.awt or java2d packages. >> >> See jdk (25?) branch: >> >> https://github.com/bourgesl/marlin-renderer/tree/jdk/src/main/java/sun/java2d >> >> Cheers, >> Laurent >> >> >> -------- Message d'origine -------- >> Le 22/08/2025 14:04, Daniel Gredler a écrit : >> >> Hi all, >> >> `BufferedImage.getRGB(int, int, int, int, int[], int, int)` is often used >> for processing of individual image pixels. A common pattern is to loop >> through each row of pixels, calling this method once per row to populate >> the row pixel `int[]` and then process it. >> >> There are many types of `BufferedImage`, but one of the most common types >> is `TYPE_INT_ARGB`. Based on a quick search on GitHub, about one third of >> all BufferedImages are of this type [1]. This is also the representation >> which `BufferedImage.getRGB(int, int, int, int, int[], int, int)` uses for >> its output. >> >> I think there may be an opportunity here (in `BufferedImage.getRGB(int, >> int, int, int, int[], int, int)`) to skip the pixel-by-pixel color model >> conversion if the `BufferedImage` is already of type `TYPE_INT_ARGB`, which >> is relatively common. See here [2] for what this optimization could look >> like. >> >> In my local testing, a simple test program [3] went from running in 220 >> seconds without the change to running in 7 seconds with the change. >> Separately, a real-world program which uses the row-by-row pixel access >> pattern went from running in 45 seconds to running in 29 seconds. >> >> Does this look like a good change to those of you who know this part of >> the code? Am I missing something that might make this dangerous or >> undesirable? Is it making too many assumptions? I know this area is fraught >> with gotchas -- color models, color spaces, strides, etc. >> >> Thanks! >> >> Daniel >> >> --- >> >> [1] >> BufferedImage.TYPE_CUSTOM: 2k >> BufferedImage.TYPE_INT_RGB: 114k >> BufferedImage.TYPE_INT_ARGB: 93k << 35% >> BufferedImage.TYPE_INT_ARGB_PRE: 5k >> BufferedImage.TYPE_INT_BGR: 4k >> BufferedImage.TYPE_3BYTE_BGR: 10k >> BufferedImage.TYPE_4BYTE_ABGR: 9k >> BufferedImage.TYPE_4BYTE_ABGR_PRE: 2k >> BufferedImage.TYPE_USHORT_565_RGB: 1k >> BufferedImage.TYPE_USHORT_555_RGB: 1k >> BufferedImage.TYPE_BYTE_GRAY: 11k >> BufferedImage.TYPE_USHORT_GRAY: 2k >> BufferedImage.TYPE_BYTE_BINARY: 5k >> BufferedImage.TYPE_BYTE_INDEXED: 3k >> Total: 262k >> >> [2] >> https://github.com/gredler/jdk/commit/b98f6cdf7573b7e89067c757890193517aeb472e >> >> [3] >> public final class PerfTest { >> public static void main(final String[] args) { >> int w = 1_000; >> int h = 1_000; >> int accumulator = 0; >> BufferedImage image = new BufferedImage(w, h, >> BufferedImage.TYPE_INT_ARGB); >> int[] row = new int[w]; >> long start = System.currentTimeMillis(); >> for (int i = 0; i < 100_000; i++) { >> for (int y = 0; y < h; y++) { >> image.getRGB(0, y, w, 1, row, 0, w); >> accumulator += row[i % w]; >> } >> } >> long end = System.currentTimeMillis(); >> System.out.println("Total time: " + ((end - start) / 1_000) + " >> seconds"); >> System.out.println("Accumulator: " + accumulator); >> } >> } >> >> >
