If that's the only change then it looks good to me...
...jim
On 08/03/2016 02:58 AM, Laurent Bourgès wrote:
Jim & Phil,
I added the following class header in Byte/Int/Float ArrayCache classes
and removed the shell script:
/*
* Note that the [BYTE/INT/FLOAT]ArrayCache files are nearly identical
except
* for a few type and name differences. Typically, the
[BYTE]ArrayCache.java file
* is edited manually and then [INT]ArrayCache.java and
[FLOAT]ArrayCache.java
* files are generated with the following command lines:
*/
// % sed -e 's/(b\yte)[ ]*//g' -e 's/b\yte/int/g' -e 's/B\yte/Int/g' <
B\yteArrayCache.java > IntArrayCache.java
// % sed -e 's/(b\yte)[ ]*/(float) /g' -e 's/b\yte/float/g' -e
's/B\yte/Float/g' < B\yteArrayCache.java > FloatArrayCache.java
I tested the commands and it works well.
Here is the updated webrev:
http://cr.openjdk.java.net/~lbourges/marlin/marlin-8159638.2/
Cheers,
Laurent
2016-08-03 2:28 GMT+02:00 Jim Graham <[email protected]
<mailto:[email protected]>>:
How about instead of the shell script we put a comment up at the top
of the files (after the copyright header), with the appropriate
command line? Something like:
/*
* Note that the Byte/Int/Float files are nearly identical except
* for a few type and name differences. Typically, the Byte version
* is edited manually and then the Int and Float versions are
* generated with the following command lines:
* % sed ... Float ...
* % sed ... Int ...
*/
The only issue is trying to word this in a way that prevents the
"Byte" in this comment from being converted. It gets even tricker
when we have the strings being substituted appear in the command
lines. Perhaps escapes would avoid the issue? And upper case?
Something like this (but it looks kind of gross):
/*
* Note that the BYTE/INT/FLOAT files are nearly identical except
* for a few type and name differences. Typically, the BYTE version
* is edited manually and then the INT and FLOAT versions are
* generated with the following command lines:
* % sed ... \b\y\t\e ... float ... \B\y\t\e ... Float ...
* % sed ... \b\y\t\e ... int ... \B\y\t\e ... Int ...
*/
A developer could either cut and paste the commands to a command
line, or write their own shell script...
...jim
On 08/02/2016 03:34 PM, Philip Race wrote:
I have not yet looked at everything but no issues except that
I find checking in the shell script a bit weird.
Not to mention its technically a "source file" so should have a
license.
-phil.
On 8/2/16, 2:56 PM, Jim Graham wrote:
Thanks Laurent,
On 08/02/2016 05:57 AM, Laurent Bourgès wrote:
Thanks for the tip, I made another webrev (for archive)
that shows the
proper diffs in ArrayCache / ArrayCacheConst:
http://cr.openjdk.java.net/~lbourges/marlin/marlin-8159638.1_bis/
<http://cr.openjdk.java.net/%7Elbourges/marlin/marlin-8159638.1_bis/>
Thanks!
In Renderer.java, you create the alphaLine and
blkFlags refs as
Clean, but then you always put them back using
indices of (0, 0) so
they will never actually be cleaned - is there a
reason you don't
just use a dirty ref there?
Both alphaLine and blkFlags arrays must be zero-filled
as these arrays
are storing accumulated values:
It is not possible to use a dirty reference in this case
as both
allocated and returned array may contain garbage data
(from the
IntArrayCache).
D'oh! I guess that was obvious. I wasn't thinking of the
fact that
dirty caches can initially return a non-zero-filled array -
the fact
that they clean on "put" is only half of their zero guarantee...
Other than that question, I don't see any problems
with the fix...
Ready to go ?
or I need another reviewer, phil ?
Ready from my end. Phil?
...jim
--
--
Laurent Bourgès
