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

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [11u] RFR 8210782: Upgrade HarfBuzz to the latest 2.3.1
From:       "Lindenmaier, Goetz" <goetz.lindenmaier () sap ! com>
Date:       2019-05-07 14:51:23
Message-ID: VI1PR02MB44630930D123506F4A8790FEEC310 () VI1PR02MB4463 ! eurprd02 ! prod ! outlook ! com
[Download RAW message or body]

Hi Christoph, 

I had a look at this change. 
The integration is good. 

I thought about the adaptions you had to do. 
They look good for the compilers you mention, and I 
would assume they also work with more recent ones.
I guess nobody uses more recent compilers on Solaris.
On AIX I know further adaptions are needed, and as 
they are missing it is ruled out anybody compiles with xlc 16.
So looks good, too.

I checked the tests, and the only somewhat related
failing one is java/awt/FontMetrics/MaxAdvanceIsMax.java,
but that is also failing without your patch.
So testing is fine.

Best regards,
  Goetz.




> -----Original Message-----
> From: jdk-updates-dev <jdk-updates-dev-bounces@openjdk.java.net> On
> Behalf Of Langer, Christoph
> Sent: Dienstag, 30. April 2019 11:26
> To: jdk-updates-dev@openjdk.java.net
> Cc: 2d-dev <2d-dev@openjdk.java.net>; build-dev@openjdk.java.net; Baesken,
> Matthias <matthias.baesken@sap.com>
> Subject: [CAUTION] [11u] RFR 8210782: Upgrade HarfBuzz to the latest 2.3.1
> 
> Hi,
> 
> please help reviewing the backport of JDK-8210782: Upgrade HarfBuzz to the
> latest 2.3.1.
> 
> This has been backported to 11.0.4-oracle already. I took the large change
> down to 11u-dev. It applies quite nicely, apart from a little issue in
> make/lib/Awt2dLibraries.gmk:
> 
> --- Awt2dLibraries.gmk
> +++ Awt2dLibraries.gmk
> @@ -613,8 +614,7 @@
>          type-limits missing-field-initializers implicit-fallthrough \
>          strict-aliasing undef unused-function, \
>      DISABLED_WARNINGS_CXX_gcc := reorder delete-non-virtual-dtor strict-
> overflow \
> -        maybe-uninitialized \
> -        missing-attributes class-memaccess, \
> +        maybe-uninitialized class-memaccess, \
>      DISABLED_WARNINGS_clang := unused-value incompatible-pointer-types \
>          tautological-constant-out-of-range-compare int-to-pointer-cast \
>          sign-compare undef missing-field-initializers, \
> 
> The original change would remove the disabling of the "missing-attributes"
> warnings, but since this warning type is not disabled in jdk11u-dev currently, I
> would just skip that diff.
> 
> With that change, most platforms did build fine, except for Solaris (where we
> use Oracle Studio 12u4 in jdk11u-dev vs Oracle Studio 12u6 in jdk/jdk) and AIX
> (xlc12 vs. xlc16).
> 
> To keep support for Oracle Studio 12u4 for Solaris, I had to remove the warning
> flag "refmemnoconstr_aggr" from line 622 (as opposed to the original change).
> Seems that it is not yet supported in OS12u4.
> 
> Furthermore, a code tweak had to be done (Thanks, Matthias, for your help
> here):
> 
> --- a/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-subset-cff-
> common.hh     Fri Mar 01 16:59:19 2019 -0800
> +++ b/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-subset-cff-
> common.hh     Mon Apr 29 16:26:41 2019 +0200
> @@ -280,6 +280,10 @@
> {
>    str_buff_t     &flatStr;
>    bool  drop_hints;
> +
> +  // Solaris: OS12u4 complains about "A class with a reference member lacks a
> user-defined constructor"
> +  // so provide the constructor
> +  flatten_param_t(str_buff_t& sbt, bool dh) : flatStr(sbt), drop_hints(dh) {}
> };
> 
> template <typename ACC, typename ENV, typename OPSET>
> @@ -305,7 +309,9 @@
>          return false;
>        cs_interpreter_t<ENV, OPSET, flatten_param_t> interp;
>        interp.env.init (str, acc, fd);
> -      flatten_param_t  param = { flat_charstrings[i], drop_hints };
> +      // Solaris: OS12u4 does not like the C++11 style init
> +      // flatten_param_t  param = { flat_charstrings[i], drop_hints };
> +      flatten_param_t  param(flat_charstrings[i], drop_hints);
>        if (unlikely (!interp.interpret (param)))
>          return false;
>      }
> 
> For AIX, this tweak had to be added (credit goes to Matthias as well):
> diff -r 2b3dbedfbfb9
> src/java.desktop/share/native/libfontmanager/harfbuzz/hb-null.hh
> --- a/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-null.hh  Fri
> Mar 01 16:59:19 2019 -0800
> +++ b/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-null.hh
> Mon Apr 29 16:26:41 2019 +0200
> @@ -83,7 +83,9 @@
> 
> template <typename T, typename V, typename B>
> struct _hb_assign
> -{ static inline void value (T &o, const V v) { o = v; } };
> +// add cast to please AIX xlc12.1
> +//{ static inline void value (T &o, const V v) { o = v; } };
> +{ static inline void value (T &o, const V v) { o = (T&) v; } };
> template <typename T, typename V>
> struct _hb_assign<T, V, _hb_bool_type<(bool) (1 + (unsigned int) T::min_size)>
> >
> { static inline void value (T &o, const V v) { o.set (v); } };
> 
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8210782
> Original Change: http://hg.openjdk.java.net/jdk/jdk/rev/7c11a7cc7c1d
> Review discussion for jdk/jdk: https://mail.openjdk.java.net/pipermail/2d-
> dev/2019-March/009914.html
> New Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8210782.jdk11u/
> 
> Thanks & Best regards
> Christoph

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