+1

Regards
Prasanta
On 9/10/2018 5:56 PM, Krishna Addepalli wrote:
Thanks for the comments Prasanta, here is the updated webrev: http://cr.openjdk.java.net/~kaddepalli/8208638/webrev02/ <http://cr.openjdk.java.net/%7Ekaddepalli/8208638/webrev02/>

Krishna

On 10-Sep-2018, at 5:20 PM, Prasanta Sadhukhan <[email protected] <mailto:[email protected]>> wrote:

yes, right. fix looks ok to me. Please change comment style in l946 and make it similar to l956. Also, l960 change "ration" to "ratio"

Regards
Prasanta
On 9/7/2018 9:00 PM, Krishna Addepalli wrote:
Hi Prasanta,
I think you are confused by the same name of the variables. newHeight/newWidth are in “updateImage” and also “ImageHandler::imageUpdate”. imageUpdate can be called from a background thread, with new information from the image being loaded, that contains the newHeight and newWidth. Along with this, the “flags” variable lets us know what properties of the image are updated. For example, ImageObserver.HEIHGT and ImageObserver.WIDTH flags being set indicates, that this update call has fetched the image properties from the file. Based on this, I cache these values, and then update them once the image load is complete.
Thanks,
Krishna
*From:*Prasanta Sadhukhan
*Sent:*Friday, September 7, 2018 7:04 PM
*To:*Krishna Addepalli<[email protected]>;[email protected] *Subject:*Re: <Swing Dev> RFR: [12] JDK-8208638: Instead of circle rendered in appl window, but ellipse is produced JEditor Pane

Hi Krishna,

We obtain newHeight and specifiedHeight by same method

newHeight = getIntAttr(HTML.Attribute.HEIGHT, -1);
and

*specifiedHeight = getIntAttr(HTML.Attribute.HEIGHT, -1);*
so I assume both will return the same value. So,

*968                         if (specifiedHeight <= 0) {*
*969                             proportion = specifiedWidth / ((double)newWidth);* *970                             newHeight = (int)(proportion * newHeight);*
*971                         }*
if specifiedHeight is <=0 then newHeight * proportion will also be -ve or 0, isn't it? Will it not cause problem? Am I missing something?

Regards
Prasanta
On 9/5/2018 4:25 PM, Krishna Addepalli wrote:

    Hi All,
    The testfix turned out to be simpler than I thought. I just had
    to query the preferred size before making the frame visible, so
    that the image is rendered on screen properly, and now Robot is
    able to pick the color.
    Here is the modified
    webrev:http://cr.openjdk.java.net/~kaddepalli/8208638/webrev01/
    <http://cr.openjdk.java.net/%7Ekaddepalli/8208638/webrev01/>
    Thanks,
    Krishna
    *From:*Krishna Addepalli
    *Sent:*Wednesday, September 5, 2018 9:41 AM
    *To:*[email protected] <mailto:[email protected]>
    *Subject:*Re: <Swing Dev> RFR: [12] JDK-8208638: Instead of
    circle rendered in appl window, but ellipse is produced JEditor Pane
    Hi All,
    I found an issue with the test, and am working on it to make it
    pass reliably. I was setting the size of the frame and then
    querying the preferred size, which makes it a non-test.
    I’m working to address it, meanwhile you could still review the
    fix itself.
    Thanks,
    Krishna
    *From:*Krishna Addepalli
    *Sent:*Wednesday, September 5, 2018 12:10 AM
    *To:*[email protected] <mailto:[email protected]>
    *Subject:*RFR: [12] JDK-8208638: Instead of circle rendered in
    appl window, but ellipse is produced JEditor Pane
    Hi All,
    Please review a fix for
    JDK-8208638:https://bugs.openjdk.java.net/browse/JDK-8208638
    Webrev:http://cr.openjdk.java.net/~kaddepalli/8208638/webrev00/
    <http://cr.openjdk.java.net/%7Ekaddepalli/8208638/webrev00/>
    JEditorPane supports loading HTML content, including images, and
    this bug is about image scaling when either of the width/height
    attributes are not specified in the HTML text.
    The problem is that scaling of the images in all cases was not
    handled properly. Here is the behavior that is observed in other
    browsers:
    1.If both width and height are specified in html, then use them
    instead.
    2.If both are not specified, then use the image dimensions.
    3.If either one is specified, then proportionately adjust the
    other dimension to preserve the aspect ratio of the image.
    ImageView.java implementation was not handling the last case,
    which is added in the fix.
    The test is also fixed now, which was earlier relying on
    Robot.getPixelColor, which does not work reliably. A happy
    alternative was found, where in the “RootView” that is rendering
    the html content is retrieved from the JEditorPane,  and its
    preferred span is queried, which should match the case specific
    width/height.
    Tested it on all platforms (Windows, Linux(Ubuntu) and Mac) and
    now it reliably passes.
    Thanks,
    Krishna





Reply via email to