Hi,
First
These comments are limited to the files that are *modified* and don't
address anything where you have deleted sandbox files and replaced them by JB
files.
And that is very hard to "diff" anyway .. but it is the biggest batch so I am
not
sure how to address it.
Can you give a detailed explanation of what is in the "new" files vs the "old"
files
and how you chose one over the other ?
So this is my quick take on just the modified files :
http://cr.openjdk.java.net/~jdv/8225160/webrev.00/make/lib/Awt2dLibraries.gmk.udiff.html
+ -framework Metal \
+ -framework MetalKit \
I thought there were no dependencies on MetalKit ?
http://cr.openjdk.java.net/~jdv/8225160/webrev.00/src/java.base/share/classes/jdk/internal/module/jdk8_packages.dat.udiff.html
+sun.java2d.metal
I am sure this is wrong. That file is a list of internal packages
that were present in JDK 8
http://cr.openjdk.java.net/~jdv/8225160/webrev.00/src/java.desktop/macosx/classes/sun/java2d/MacosxSurfaceManagerFactory.java.udiff.html
JFYI I am not sure I like *either* of the pieces of code here.
The deleted one or the new one.
By which I mean the logic of
if (metal) {
return MetalSM()
} else {
return CGLSM();
}
both here, and where similar logic appears in other files.
We should be installing something that always returns what we need based
on choice of pipeline at start up, not doing error prone repeating of
the decision.
But that is something to discuss later since it isn't a merge issue.
http://cr.openjdk.java.net/~jdv/8225160/webrev.00/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.h.udiff.html
+#import<Metal/Metal.h>
+#import<MetalKit/MetalKit.h>
I am failing to locate what needs these new imports ??
Same here
http://cr.openjdk.java.net/~jdv/8225160/webrev.00/src/java.desktop/macosx/native/libawt_lwawt/awt/CGraphicsDevice.m.udiff.html
http://cr.openjdk.java.net/~jdv/8225160/webrev.00/src/java.desktop/share/classes/sun/java2d/loops/Blit.java.udiff.html
I see this pattern being added to the Trace* classes.
At first it wasn't apparent from the diff it is the Trace classes,
so this is something we can discuss but it may need to be measured
to see if it corrupts the very data you want.
{
+ if ((traceflags& TRACEPTIME) == 0) {
tracePrimitive(target);
+ }
+ long time = System.nanoTime();
target.Blit(src, dst, comp, clip,
srcx, srcy, dstx, dsty, width, height);
+ tracePrimitiveTime(target, System.nanoTime() - time);
-phil
On 6/17/19, 2:19 AM, Jayathirth Rao wrote:
Hello All,
Please review the below code changes:
JBS : https://bugs.openjdk.java.net/browse/JDK-8225160
We have merged most of the code provided for review by JetBrains for
JDK-8220154 <https://bugs.openjdk.java.net/browse/JDK-8220154> to the
jdk/sandbox(http://hg.openjdk.java.net/jdk/sandbox).
Corresponding webrev for the change is
http://cr.openjdk.java.net/~jdv/8225160/webrev.00/
<http://cr.openjdk.java.net/%7Ejdv/8225160/webrev.00/> . This
webrev has JetBrains code from JDK-8220154
<https://bugs.openjdk.java.net/browse/JDK-8220154> and our delta
modification[1] over jdk/sandbox metal-prototype-branch
For additional info :
1. A webrev relative to the jdk/client codebase - this has JetBrains
webrev for JDK-8220154
<https://bugs.openjdk.java.net/browse/JDK-8220154> and our delta
modification
http://cr.openjdk.java.net/~aghaisas/8225160/JB_plus_delta/webrev.1/
<http://cr.openjdk.java.net/%7Eaghaisas/8225160/JB_plus_delta/webrev.1/>
2. A webrev showing our delta modifications over JetBrains webrev for
JDK-8220154 <https://bugs.openjdk.java.net/browse/JDK-8220154>
http://cr.openjdk.java.net/~aghaisas/8225160/webrev.1/
<http://cr.openjdk.java.net/%7Eaghaisas/8225160/webrev.1/>
[1] Delta modification: At native rendering side Ajit has added delta
modifications to implement FillRect and DrawParallelogram logic using
JetBrains initialisation logic.
Apart from how we initialise GraphicsConfig and native rendering most
of the upper level logic is similar so we have taken most of the
JetBrains code with MTL*** files. Also native rendering state
management code from JetBrains we have merged.
Comparison of code at GraphicsConfig, SurfaceData and Layer is
captured under subtask :
https://bugs.openjdk.java.net/browse/JDK-8225165 .
Thanks,
Jay