[OpenJDK 2D-Dev] RFR: 8149815: Misleading

[Brian Burkhalter]

Hello Bharath,

Thank you for your comments.

On May 10, 2016, at 8:45 AM, Bluv Nallakaluva <bharath.nallakaluva at oracle.com> wrote:

> Please find my observations below on this.
> 
> Even when hasDirectory is true for the TIFFField , the node returned by TIFFField#getAsNativeNode does not return "TIFFIFD" value when the method getNodeName is invoked on it. Please refer to old attached test case here : https://bugs.openjdk.java.net/browse/JDK-8149815.

When I run the test case GetAsNativeNode attached to https://bugs.openjdk.java.net/browse/JDK-8149815 against my current clone it produces no output.

> The TIFFField constructed with a TIFFDirectory argument returns "true” when hasDirectory method is invoked on that instance.
> 
> The root cause might be originating from the internal api  "com.sun.imageio.plugins.tiff.TIFFFieldNode#getNodeName.” This method is using the check return f.getData() instanceof TIFFDirectory ? "TIFFIFD" : "TIFFField  which is not quite true because f.getData cannot be an instance of  TIFFDirectory as per the current API.
> 
> I believe the fix for this should be "return f.hasDirectory && f.getTag().isIFDPointer ? "TIFFIFD" :"TIFFField""

With this patch http://cr.openjdk.java.net/~bpb/8149815/webrev.00/ applied, the TIFFField.getAsNativeNode() is
    /**
     * Returns the {@code TIFFField} as a node named either
     * <tt>"TIFFField"</tt> or <tt>"TIFFIFD"</tt> as described in the
     * TIFF native image metadata specification. The node will be named
     * <tt>"TIFFIFD"</tt> if and only if {@link #hasDirectory()} returns
     * {@code true}, or the field's data object is an instance of
     * {@link TIFFDirectory}.
     *
     * @return a {@code Node} named <tt>"TIFFField"</tt> or
     * <tt>"TIFFIFD"</tt>.
     */
    public Node getAsNativeNode() {
        return new TIFFFieldNode(this);
    }
and the TIFFFieldNode constructor uses
    private static String getNodeName(TIFFField f) {
        return (f.hasDirectory() || f.getData() instanceof TIFFDirectory) ?
            "TIFFIFD" : "TIFFField";
    }

I think you do have a point but I do not think it is entirely correct. I would propose instead the following version of the above two methods:
    /**
     * Returns the {@code TIFFField} as a node named either
     * <tt>"TIFFField"</tt> or <tt>"TIFFIFD"</tt> as described in the
     * TIFF native image metadata specification. The node will be named
     * <tt>"TIFFIFD"</tt> if and only if {@link #hasDirectory()} returns
     * {@code true} and the field’s type is either {@link TIFFTag#TIFF_LONG}
     * or {@link TIFFTag#TIFF_IFD_POINTER}.
     *
     * @return a {@code Node} named <tt>"TIFFField"</tt> or
     * <tt>"TIFFIFD"</tt>.
     */
    public Node getAsNativeNode() {
        return new TIFFFieldNode(this);
    }

    private static String getNodeName(TIFFField f) {
        int type = f.getType();
        return (f.hasDirectory() &&
            (type == TIFFTag.TIFF_LONG || type == TIFFTag.TIFF_IFD_POINTER)) ?
            "TIFFIFD" : "TIFFField";
    }
Thanks,

Brian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160517/eec00739/attachment.html>