Hi Phil, Thanks for your inputs.
I have updated the code to reflect the changes mentioned by you and changed test case to use ByteArrayInput/OutputStream. Please find updated webrev for review: http://cr.openjdk.java.net/~jdv/6788458/webrev.08/ Regards, Jay -----Original Message----- From: Phil Race Sent: Wednesday, April 11, 2018 10:23 PM To: Jayathirth D V; Prahalad Kumar Narayanan; 2d-dev Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images http://cr.openjdk.java.net/~jdv/6788458/webrev.07/src/java.desktop/share/classes/com/sun/imageio/plugins/png/PNGMetadata.java.udiff.html There's an extra line added at the end which you can remove. Why do the tests needs to create temp disk files ? Can't we do this all with ByteArrayInput/OutputStream ? Less worry about clean up there. I note that this 286 boolean tRNSTransparentPixelPresent = 1287 theImage.getSampleModel().getNumBands() == inputBands + 1 && 1288 metadata.hasTransparentColor(); is evaluated for each row when I expect it only needs to be evaluated once for the whole decode pass. But the overhed is presumably low and it is next to the use so I think it should be OK as is. However the code that does the transparent per-pixel settings does seem needlessly wasteful. These can be moved outside at least the entire row decode : final int[] temp = new int[inputBands + 1]; final int opaque = (bitDepth < 16) ? 255 : 65535; Note that I added final to them .. Also array accesses are (relatively) slow but since you need an array to pass to setPixel and there's only two accesses I don't see how we can improve on that here by assigning the values from the ps array to local variables. -phil. On 04/09/2018 11:19 PM, Jayathirth D V wrote: > HI Prahalad, > > Thanks for your inputs. > > Regarding- > Few minor corrections to PNGImageReader- . Line: 1310 > . This missed my eye in the last review. > . We could create int[] temp outside the for loop. I tried this with > your changes & it works. > . It's an optimization so that we don't end up creating int[] array for > every pixel in the row. > > Before sending webrev.06, I actually made similar changes of moving temp[] > creation and/or calculating "opaque" to outside of for loop. > But this will cause creation of temp[] and/ or calculation of "opaque" for > each row in all the cases where there is no RGB/Gray & tRNS combination. So I > reverted those change before sending webrev.06. > I would like to keep all the calculation specific to RGB/Gray & tRNS > combination in its own code flow. > > Other modifications are made. > Please find updated webrev for review: > http://cr.openjdk.java.net/~jdv/6788458/webrev.07/ > > Thanks, > Jay > > -----Original Message----- > From: Prahalad Kumar Narayanan > Sent: Monday, April 09, 2018 2:23 PM > To: Jayathirth D V; 2d-dev > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hello Jay > > I looked into the latest code changes. > The code changes are good. > > Few minor corrections to PNGImageReader- . Line: 1310 > . This missed my eye in the last review. > . We could create int[] temp outside the for loop. I tried this with > your changes & it works. > . It's an optimization so that we don't end up creating int[] array for > every pixel in the row. > > . Line: 1320, 1329 > . Align the opening { on the same line as the if statement. > > . Line: 1479 > . Correct the indentation of the expression within if condition. > . A suggestion that could help: > if ((theImage.getSampleModel().getNumBands() > == inputBandsForColorType[colorType] + 1) > && metadata.hasTransparentColor()) { > // code block > checkReadParamBandSettings(param, > ... > } > > Thank you > Have a good day > > Prahalad N. > > -----Original Message----- > From: Jayathirth D V > Sent: Friday, April 06, 2018 5:19 PM > To: Prahalad Kumar Narayanan; 2d-dev > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hi Prahalad, > > Thanks for your inputs. > > Regarding : > File: PNGImageReader.java > Line: 1329, 1342 > . The else block for if (check for transparent pixel) is redundant > across both PNG_COLOR_RGB and PNG_COLOR_GRAY. > . We could use Arrays.fill with opaque value and set the alpha to 0 if > pixel is transparent. > int[] temp = new int[inputBands + 1]; > int opaque = (bitDepth < 16) ? 255 : 65535; > Arrays.fill(temp, opaque); > . All subsequent operations checking for transparent color value and > setting alpha to 0 would be required. > > I think we should not use Arrays.fill() at this place and assign values to > all the channels. It is a per pixel operation and we would be writing data > twice. > > Instead of using Arrays.fill(), I thought of just assigning value to alpha > channel: > int[] temp = new int[inputBands + 1]; > temp[inputBands] = (bitDepth < 16) ? 255 : 65535; if > (metadata.tRNS_colorType == PNG_COLOR_RGB) { > > I think even doing this is not ideal because anyway for all the pixels we > will be checking pixel data with tRNS data, and assign value in alpha > channel. There is no need for us to assign some value and override it again > later if a condition satisfies. > So I am assigning opaque value to alpha channel at same place. But I have > reduced the LOC by using ternary operator for determining the opaque value > for alpha channel. > > All other changes are updated. > Please find updated webrev for review: > http://cr.openjdk.java.net/~jdv/6788458/webrev.06/ > > Thanks, > Jay > > -----Original Message----- > From: Prahalad Kumar Narayanan > Sent: Friday, April 06, 2018 1:42 PM > To: Jayathirth D V; 2d-dev > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hello Jay > > Good day to you. > > I looked into the latest changes. > The code changes are nearly complete. Just a few tweaks. > > File: PNGImageReader.java > Line: 1280 > Rephrase from: > /* > * In case of colortype PNG_COLOR_RGB or PNG_COLOR_GRAY > * if we have transparent pixel information from tRNS chunk > * we need to consider that also and store proper information > * in alpha channel. > * > * Also we create destination image with extra alpha channel > * in getImageTypes() when we have tRNS chunk for colorType > * PNG_COLOR_RGB or PNG_COLOR_GRAY. > */ > Rephrase to: > /* > * For PNG images of color type PNG_COLOR_RGB or > PNG_COLOR_GRAY > * that contain a specific transparent color (given by tRNS > * chunk), we compare the decoded pixel color with the color > * given by tRNS chunk to set the alpha on the destination. > */ > > File: PNGImageReader.java > Line: 1588 > Rephrase from: > /* > * In case of PNG_COLOR_RGB or PNG_COLOR_GRAY, if we > * have transparent pixel information in tRNS chunk > * we create destination image having alpha channel. > */ > > Rephrase to: > /* > * For PNG images of color type PNG_COLOR_RGB or PNG_COLOR_GRAY that > * contain a specific transparent color (given by tRNS chunk), we add > * ImageTypeSpecifier(s) that support transparency to the list of > * supported image types. > */ > > File: PNGImageReader.java > Line(s): 1290, 1493, 1596, 1619 > . The lines mentioned above check whether metadata has specific > transparent color using- > metadata.tRNS_present && > (metadata.tRNS_colorType == PNG_COLOR_RGB || > metadata.tRNS_colorType == PNG_COLOR_GRAY) > > . The above check is not only redundant but also metadata specific. > . We could move the code to a common method in PNGMetadata- > boolean hasTransparentColor() { > return tRNS_present > && (tRNS_colorType == PNG_COLOR_RGB > || tRNS_colorType == PNG_COLOR_GRAY); > } > . I understand 1596 and 1619 check for specific color type but they can > be avoided with this method as well. > . As per the specification, tRNS values depend on the image's color type. > . This will reduce the complex check from- > if (theImage.getSampleModel().getNumBands() == > inputBandsForColorType[colorType] + 1 && > metadata.tRNS_present && > (metadata.tRNS_colorType == PNG_COLOR_RGB || > metadata.tRNS_colorType == PNG_COLOR_GRAY)) > { > to- > if (metadata.hasTransparentColor() > && (theImage.getSampleModel().getNumBands() == > inputBandsForColorType[colorType] + 1)) { > > File: PNGImageReader.java > Line: 1329, 1342 > . The else block for if (check for transparent pixel) is redundant > across both PNG_COLOR_RGB and PNG_COLOR_GRAY. > . We could use Arrays.fill with opaque value and set the alpha to 0 if > pixel is transparent. > int[] temp = new int[inputBands + 1]; > int opaque = (bitDepth < 16) ? 255 : 65535; > Arrays.fill(temp, opaque); > . All subsequent operations checking for transparent color value and > setting alpha to 0 would be required. > > File: PNGImageReader.java > All modified Lines: > . The opening braces '{' can appear in the new line. Some engineers do > follow this. > . Since the rest of the code aligns opening braces in the same line as > the ' if ' statement we could follow the same for code readability. > > Test File: ReadPngGrayImageWithTRNSChunk.java and > ReadPngRGBImageWithTRNSChunk.java > . The finally blocks should check whether the objects- ImageOutputStream > and File, are not null. > . The call to directory(while is a File).delete() is not required in my > view. Verify this once. > . Dispose the writer after the write operation is complete. > > Thank you > Have a good day > > Prahalad N. > > -----Original Message----- > From: Jayathirth D V > Sent: Thursday, April 05, 2018 3:26 PM > To: Prahalad Kumar Narayanan; 2d-dev > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hello Prahalad, > > Thank you for the inputs. > > I gave updated the code to not change ImageTypeSpecifier of passRow. Now > while storing the pixel values into imRas we comparing the pixel RGB/Gray > values to tRNS RGB/Gray values and storing appropriate value for alpha > channel. > I have added support to use tRNS_Gray value when IHDR color type is > PNG_COLOR_GRAY - We are now creating 2 channel destination image whenever we > see that tRNS chunk is present for PNG_COLOR_GRAY in getImageTypes(). > isTransparentPixelPresent() code is removed and we are using available tRNS > properties. > > I have merged previous test cases. Now we have 2 test cases each verifying > the code changes for RGB & Gray image for 8 & 16 bit depth values. > Please find updated webrev for review: > http://cr.openjdk.java.net/~jdv/6788458/webrev.05/ > > Thanks, > Jay > > -----Original Message----- > From: Prahalad Kumar Narayanan > Sent: Monday, April 02, 2018 12:03 PM > To: Krishna Addepalli; Jayathirth D V; 2d-dev > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hello Jay > > Good day to you. > > I looked into the latest code changes. > Please find my review observation & suggestions herewith. > > My understanding of problem statement: > . As I understand, our PNGImageReader parses tRNS chunk information from > metadata whenever ignoreMetadata is false or paletted image is decoded. > . But doesn't use the transparency information contained in the chunk to > return BufferedImages with alpha/ transparency. > > Appropriate Changes: > > File: PNGImageReader.java, > Method: Iterator<ImageTypeSpecifier> getImageTypes > Line: 1633 > . This method is internally invoked while creating BufferedImage for > destination at Line 1409: > theImage = getDestination(param, > getImageTypes(0), > width, > height); > . The move to add ImageTypeSpecifier based on > BufferedImage.TYPE_4BYTE_ABGR as the first element (index: 0) of the list is > good. > > File: PNGImageReader.java > Method: readMetadata > Line: 731 > . The if check for parsing tRNS chunk even when ignoreMetadata is set to > true is good. > . The chunk's information will be needed. > > Other Observation & Suggestions: > > File: PNGImageReader.java, > Method: decodePass > Line: (1088 - 1112), (1277-1327) > . In the code chunks of listed line numbers, you have modified passRow's > internal type- ImageTypeSpecifier, and thus its manipulation logic as well. > . The changes are not required in my view. Reasons are- > . passRow corresponds to the data from input image source. > . imgRas corresponds to the destination buffered image. > . To return a BufferedImage with alpha/ transparency, it would > suffice to update imgRas with alpha component at Line 1371. > imRas.setPixel(dstX, dstY, ps); // if ps contains alpha, it > would suffice the need. > . However, the proposed changes modify how we interpret the source > through passRow. > . To set alpha component on imgRas, we would require comparison of > color components present in passRow with that of tRNS chunk. > . But, passRow 's internal type- ImageTypeSpecifier need not be > changed. This way most of the complex code changes can be avoided. > > File: PNGImageReader.java, > Method: isTransparentPixelPresent > Line: 1545 > . The logic of this method considers both input image source and > destination bands to decide whether transparency is available. > . In my view, The method should consider only the alpha in input image > source and not the destination. > . Here are code points where the method is invoked > a) 1089 : To create a writable raster- passRow. passRow > corresponds to the data of image source. > b) 1279 : To update the passRow's databuffer. passRow corresponds > to the data of image source. > c) 1512 : To pass appropriate band length to > checkParamBandSettings. (Hardcoded value: 4) > d) 1632 & 1648 : To update ArrayList<ImageTypeSpecifier> l based > on presence of tRNS in image source. > . Each of the locations except (c) pertain to image source and not the > destination. > . One possible solution would be to discard this method and use the > existing flag tRNS_present at (c). > > . Besides, The proposed changes don't address images with gray scale > with alpha in tRNS chunk. > . Your first email reads- "PNG_COLOR_GRAY image with tRNS > chunk(which is very rare)" > . Just curious to know if there 's any basis for this observation ? > . If we consider suggestions on decodePass method, I believe, we could > support setting alpha values for grayscale images as well. > > Line: 1555 > . Please use tRNS_colorType together with tRNS_present flag. > > File: PNGImageReader.java, > Method: readMetadata > Line: 701 > Rephrase From: > * Optimization: We can skip reading metadata if ignoreMetadata > * flag is set and colorType is not PNG_COLOR_PALETTE. But we need > * to parse only the tRNS chunk even in the case where IHDR colortype > * is not PNG_COLOR_PALETTE, because we need tRNS chunk transparent > * pixel information for PNG_COLOR_RGB while storing the pixel data > * in decodePass(). > To: > * Optimization: We can skip reading metadata if ignoreMetadata > * flag is set and colorType is not PNG_COLOR_PALETTE. However, > * we parse tRNS chunk to retrieve the transparent color from the > * metadata irrespective of the colorType. Doing so, helps > * PNGImageReader to appropriately identify and set transparent > * pixels in the decoded image. > > File: PNGMetadata.java > Line: 271 > . Reset the tRNS_colorType to -1 in the reset() method. > . This will not concern if tRNS_colorType is used in conjunction with > tRNS_present flag. > . However, the new method isTransparentPixelAvailable() uses > tRNS_colorType directly. > . When the same ImageReader is used to read multiple PNG images, this > could pose a problem. > > Thank you > Have a good day > > Prahalad N. > > > ----- Original Message ----- > From: Krishna Addepalli > Sent: Monday, April 02, 2018 11:40 AM > To: Jayathirth D V; 2d-dev > Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hmmm, thanks for the clarification, but this raises one more question: Now > that you are initializing the colorType to an invalid value, do you need to > check appropriately in all the places where the color is being used? > Otherwise, it might lead to undefined behavior. > Also, I would like you to either add a comment at the point of initialization > or better yet, define another static constant of "UNDEFINED", and then set > the tRNS_colorType to this value, so that the code reads correct naturally > without any comments. > > Thanks, > Krishna > > From: Jayathirth D V > Sent: Monday, April 2, 2018 11:33 AM > To: Krishna Addepalli <[email protected]>; 2d-dev > <[email protected]> > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hi Krishna, > > The constant values for different color types is : > static final int PNG_COLOR_GRAY = 0; > static final int PNG_COLOR_RGB = 2; > static final int PNG_COLOR_PALETTE = 3; > static final int PNG_COLOR_GRAY_ALPHA = 4; > static final int PNG_COLOR_RGB_ALPHA = 6; > > Since tRNS_colorType is used to hold one of the above mentioned values if we > don't initialize it to some unused value(here we are using -1) we will be > representing PNG_COLOR_GRAY by default. > By default the value will be -1 after the change and after we parse tRNS > chunk it will contain appropriate value. The initialization of tRNS_colorType > change can be considered as newly added check. > > Thanks, > Jay > > From: Krishna Addepalli > Sent: Monday, April 02, 2018 9:56 AM > To: Jayathirth D V; 2d-dev > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hi Jay, > > The changes look fine. However, I have one more question: Why did you have to > explicitly specify the initial value to "tRNS_colorType" in PNGMetadata.java? > How is it affecting your implementation? > > Thanks, > Krishna > > From: Jayathirth D V > Sent: Wednesday, March 28, 2018 1:43 PM > To: Krishna Addepalli <[email protected]>; 2d-dev > <[email protected]> > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hi Krishna, > > Thanks for providing the inputs. > > 1. I suggest to rename considerTransparentPixel as isAlphaPresent. > As discussed I have added new name as isTransparentPixelPresent() > > 2. You can refactor the function body as below: > Return ((destinationBands == null || > destinationBands.length == 4) && > metadata.tRNS_colorType == PNG_COLOR_RGB); > > Changes are made. > > 3. From line 1088 through 1113, I see some repeated calculations like > bytesPerRow etc. Why can't we reuse the previously defined variables? Apart > from destBands, all the other calculations look similar and repeated. > > Previous to this change all the values like inputsBands, bytesPerRow, > eltsPerRow were same between the decoded output and destination image. > Now because we are changing only the destination image due to the presence of > transparent pixel, we need both these set of values. inputsBands, > bytesPerRow, eltsPerRow will be used for decoded output and destBands, > destEltsPerRow for destination image. Its better if don't mingle code flow or > variables between these two code paths. > > 4. When you are copying the pixels to a writable raster, at line 1273, you > could reduce the repetition of the lines, by just having one statement inside > if as below: > for (int i = 0; i < passWidth; i++) { > byteData[destidx] = curr[srcidx]; > byteData[destidx + 1] = curr[srcidx + 1]; > byteData[destidx + 2] = curr[srcidx + 2]; > if (curr[srcidx] == (byte)metadata.tRNS_red && > curr[srcidx + 1] == (byte)metadata.tRNS_green && > curr[srcidx + 2] == (byte)metadata.tRNS_blue) > { > byteData[destidx + 3] = (byte)0; > } else { > byteData[destidx + 3] = (byte)255; > } > srcidx += 3; > destidx += 4; > } > Same thing can be done for the loop that executes for 16bit pixel data. > > Changes are made. > > > Please find updated webrev for review : > http://cr.openjdk.java.net/~jdv/6788458/webrev.03/ > > Thanks, > Jay > > From: Krishna Addepalli > Sent: Wednesday, March 28, 2018 11:52 AM > To: Jayathirth D V; 2d-dev > Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hi Jay, > > I have some points as below: > 1. I suggest to rename considerTransparentPixel as isAlphaPresent. > 2. You can refactor the function body as below: > Return ((destinationBands == null || > destinationBands.length == 4) && > metadata.tRNS_colorType == PNG_COLOR_RGB); 3. From line 1088 > through 1113, I see some repeated calculations like bytesPerRow etc. Why > can't we reuse the previously defined variables? Apart from destBands, all > the other calculations look similar and repeated. > 4. When you are copying the pixels to a writable raster, at line 1273, you > could reduce the repetition of the lines, by just having one statement inside > if as below: > for (int i = 0; i < passWidth; i++) { > byteData[destidx] = curr[srcidx]; > byteData[destidx + 1] = curr[srcidx + 1]; > byteData[destidx + 2] = curr[srcidx + 2]; > if (curr[srcidx] == (byte)metadata.tRNS_red && > curr[srcidx + 1] == (byte)metadata.tRNS_green && > curr[srcidx + 2] == (byte)metadata.tRNS_blue) > { > byteData[destidx + 3] = (byte)0; > } else { > byteData[destidx + 3] = (byte)255; > } > srcidx += 3; > destidx += 4; > } > Same thing can be done for the loop that executes for 16bit pixel data. > > > I haven't yet checked the test cases. > > Thanks, > Krishna > > > From: Jayathirth D V > Sent: Wednesday, March 28, 2018 9:52 AM > To: 2d-dev <[email protected]> > Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hello All, > > I have enhanced both the test case to verify pixels which not only match tRNS > transparent pixel data but also for them which doesn't match tRNS transparent > pixel data. > > Please find updated webrev for review: > http://cr.openjdk.java.net/~jdv/6788458/webrev.02/ > > Thanks, > Jay > > From: Jayathirth D V > Sent: Wednesday, March 28, 2018 8:28 AM > To: 2d-dev > Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader > ignores tRNS chunk while reading non-indexed RGB images > > Hello All, > > I just realized that the test case Read16BitPNGWithTRNSChunk.java is creating > (50, 50) image(which was done while debugging the issue), which is not needed > as we need single pixel to verify the fix as it is done in > Read8BitPNGWithTRNSChunk.java. I have updated Read16BitPNGWithTRNSChunk.java > to reflect this small change. > > Please find updated webrev for review: > http://cr.openjdk.java.net/~jdv/6788458/webrev.01/ > > Thanks, > Jay > > From: Jayathirth D V > Sent: Tuesday, March 27, 2018 6:51 PM > To: 2d-dev > Subject: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores > tRNS chunk while reading non-indexed RGB images > > Hello All, > > Please review the following solution in JDK11 : > > Bug : https://bugs.openjdk.java.net/browse/JDK-6788458 > Webrev : http://cr.openjdk.java.net/~jdv/6788458/webrev.00/ > > Issue: When we try to read non-indexed RGB PNG image having transparent pixel > information in tRNS chunk, ImageIO.PNGImageReader ignores the transparent > pixel information. If we use Toolkit.getDefaultToolkit().createImage() it > doesn't ignore the transparent pixel information. > > Root cause: In ImageIO.PNGImageReader() we store tRNS chunk values in > readMetadata() -> parse_tRNS_chunk (), but we never use the tRNS R, G, B > value to compare with decoded image information. Also in > ImageIO.PNGImageReader() for IHDR colortype RGB we use only 3 channel > destination BufferedImage. So even if we use the tRNS chunk value we need > destination BufferedImage of 4 channels to represent transparency. > > Solution: While assigning destination image in PNGImageReader.getImageTypes() > if we know that PNG image is of type RGB and has tRNS chunk then we need to > assign a destination image having 4 channels. After that in decodePass() we > need to create 4 channel destination raster and while we store decoded image > information into the destination BufferedImage we need to compare decoded R, > G, B values with tRNS R, G, B values and store appropriate alpha channel > value. > > Also we use 4 channel destination image in case of RGB image only when tRNS > chunk is present and ImageReadParam.destinationBands is null or > ImageReadParam.destinationBands is equal to 4. > > One more change is that we have optimization in PNGImageReader.readMetadata() > where if ignoreMetadata is true and IHDR colortype is not of type > PNG_COLOR_PALETTE, we ignore all the chunk data and just try to find first > IDAT chunk. But if we need tRNS chunk values in case of IHDR colortype > PNG_COLOR_RGB we need to parse tNRS chunk also. So in readMetadata() also > appropriate changes are made. > > Note : Initially the enhancement request was present only for 8 bit RGB PNG > images but after making more modifications realized that we can add support > for 16 bit RGB image also by making similar incremental changes. The tRNS > chunk value is still ignored for Gray PNG image. If we need to support > PNG_COLOR_GRAY image with tRNS chunk(which is very rare) we can take that up > in a separate bug as it needs different set of changes. > > Thanks, > Jay
