On 2/20/2015 1:48 PM, Alan Bateman wrote:
Mandy pointed to me to this review thread, I'm not otherwise on 2d-dev.
First, just to put more context on this. The API for charsets is
java.nio.charsets. It has a service provider interface and the module
jdk.charsets is intended to be a service provider module with the
double byte / extended charsets. Nothing should be directly linking to
or accessing the classes in this module. So it's good to see the font
code being re-examined as this is a dependency that needs to be fixed.
I skimmed over the webrev and have a few questions:
1. For the record, can you explain why the X11 charsets can't move to
jdk.charsets? I have a vague recollection that they aren't standard
but I can't recall any details.
They aren't standard. And are (mostly) for X11 only. And if they did
then as I understand
then we'd still have a dependency on the jdk.charsets module which is
one of the motivations
for this, is it not ?
2. sun.nio.cs.ext.EUC_TWMapping is generated in the build and it
doesn't look right to me to check in a copy. Same comment on EUC_KR
and EUC_CN as it looks like they have been copied into X11KSC5601 and
X11GB2312. Have to look at generating these in the build instead?
Of course I considered this but I don't see the need. They are perfectly
stable
for our needs, and they aren't all completely identical -so I'd need
new code to generate
them and the work to get this done is already quite sufficient without
creating more
that isn't needed.
3. There is also a copy of sun.nio.cs.DoubleByte in the webrev. This
used to be in sun.nio.cs.ext (and hence jdk.charsets) so maybe this
patch started out before Sherman pushed the changes for JDK-8073152.
Yes, I just learned of this but it doesn't make any difference to this patch
since its not moved to a public location.
4. SurrogateParser also looks familiar, is this a copy of
sun.nio.cs.Surrogate.Parser?
Yes. What can I say ? There's no other answer here.
5. I see HKSCS is being removed, can you say a bit more about that?
Its now standard and we can use it directly.
I don't have any other comments at this time, I think the main thing
to understand here is why it is necessary to check in copies of
generated charsets and to see if we can avoid having copies of
DoubleByte and Surrogate.Parser.
We can't avoid copies of these classes unless some one promotes them to
exported API.
-phil.
-Alan
