[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] RFR: (JDK-8068619) - remove internal layout function to match ICU and harfbuzz
From:       Phil Race <philip.race () oracle ! com>
Date:       2015-03-05 23:33:01
Message-ID: 54F8E7AD.80705 () oracle ! com
[Download RAW message or body]

+1

-phil.

On 3/5/2015 3:28 PM, Steven Loomis wrote:
> Bug:https://bugs.openjdk.java.net/browse/JDK-8068619
> webrev:http://cr.openjdk.java.net/~srl/8068619/webrev.3/
>
> No change from before, just using cr.openjdk.java.net to host
>
> NB ICU (and Harfbuzz) have already applied this change.
> -s
>
> On 2/26/2015 11:18 AM, Steven Loomis wrote:
>> Should be a fairly simple request, deletes a function (unused by the 
>> JDK) which was removed from ICU and harfbuzz for security purposes.
>> cc: to the harfbuzz-dev list, I am requesting to land this in jdk9 
>> first as it is not strictly harfbuzz related.
>>
>> To be removed:
>>
>>     virtual const void *LEFontInstance::getFontTable(LETag tableTag) 
>> const = 0;
>>
>> This version returns the address of memory without any size indication.
>>
>> Rationale is for syncup with ICU and with Harfbuzz's version of this 
>> API.
>> The version used by the JDK takes a size_t length.
>> Again this function is NOT called  anywhere in the JDK, and is 
>> internal as far as JDK's API goes.
>>
>>
>> Bug:https://bugs.openjdk.java.net/browse/JDK-8068619
>>
>> cr.openjdk.java.net locked me out after one successful post.  So the 
>> main .h change  is at this URL: 
>> http://cr.openjdk.java.net/~srl/8068619/webrev.0/
>>
>> And the complete patch ( webrev.2) is below.. I will repost it to cr 
>> once cr lets me back in.
>>
>> ------------------
>>
>> --- 
>> old/jdk/src/java.desktop/share/native/libfontmanager/FontInstanceAdapter.cpp 
>> 2015-02-26 11:12:45.113980779 -0800
>> +++ 
>> new/jdk/src/java.desktop/share/native/libfontmanager/FontInstanceAdapter.cpp 
>> 2015-02-26 11:12:44.974981686 -0800
>> @@ -67,12 +67,6 @@
>>  };
>>
>>
>> -const void *FontInstanceAdapter::getFontTable(LETag tableTag) const
>> -{
>> -  size_t ignored = 0;
>> -  return getFontTable(tableTag, ignored);
>> -}
>> -
>>  static const LETag cacheMap[LAYOUTCACHE_ENTRIES] = {
>>    GPOS_TAG, GDEF_TAG, GSUB_TAG, MORT_TAG, MORX_TAG, KERN_TAG
>>  };
>> --- 
>> old/jdk/src/java.desktop/share/native/libfontmanager/FontInstanceAdapter.h 
>> 2015-02-26 11:12:45.548977941 -0800
>> +++ 
>> new/jdk/src/java.desktop/share/native/libfontmanager/FontInstanceAdapter.h 
>> 2015-02-26 11:12:45.394978946 -0800
>> @@ -85,7 +85,6 @@
>>
>>      // tables are cached with the native font scaler data
>>      // only supports gsub, gpos, gdef, mort tables at present
>> -    virtual const void *getFontTable(LETag tableTag) const;
>>      virtual const void *getFontTable(LETag tableTag, size_t &len) 
>> const;
>>
>>      virtual void *getKernPairs() const {
>> --- 
>> old/jdk/src/java.desktop/share/native/libfontmanager/layout/LEFontInstance.h 
>> 2015-02-26 11:12:46.042974718 -0800
>> +++ 
>> new/jdk/src/java.desktop/share/native/libfontmanager/layout/LEFontInstance.h 
>> 2015-02-26 11:12:45.881975768 -0800
>> @@ -181,28 +181,6 @@
>>       *
>>       * Subclasses which represent composite fonts should always 
>> return <code>NULL</code>.
>>       *
>> -     * Note that implementing this function does not allow for range 
>> checking.
>> -     * Subclasses that desire the safety of range checking must 
>> implement the
>> -     * variation which has a length parameter.
>> -     *
>> -     * @param tableTag - the four byte table tag. (e.g. 'cmap')
>> -     *
>> -     * @return the address of the table in memory, or <code>NULL</code>
>> -     *         if the table doesn't exist.
>> -     *
>> -     * @stable ICU 2.8
>> -     */
>> -    virtual const void *getFontTable(LETag tableTag) const = 0;
>> -
>> -    /**
>> -     * This method reads a table from the font. Note that in general,
>> -     * it only makes sense to call this method on an 
>> <code>LEFontInstance</code>
>> -     * which represents a physical font - i.e. one which has been 
>> returned by
>> -     * <code>getSubFont()</code>. This is because each subfont in a 
>> composite font
>> -     * will have different tables, and there's no way to know which 
>> subfont to access.
>> -     *
>> -     * Subclasses which represent composite fonts should always 
>> return <code>NULL</code>.
>> -     *
>>       * This version sets a length, for range checking.
>>       * Note that range checking can only be accomplished if this 
>> function is
>>       * implemented in subclasses.
>> @@ -213,7 +191,7 @@
>>       *         if the table doesn't exist.
>>       * @internal
>>       */
>> -    virtual const void* getFontTable(LETag tableTag, size_t &length) 
>> const { length=-1; return getFontTable(tableTag); } /* -1 = unknown 
>> length */
>> +    virtual const void* getFontTable(LETag tableTag, size_t &length) 
>> const = 0;
>>
>>      virtual void *getKernPairs() const = 0;
>>      virtual void  setKernPairs(void *pairs) const = 0;
>

[prev in list] [next in list] [prev in thread] [next in thread]