Hi Laurent,

On 11/23/15 9:02 AM, Laurent Bourgès wrote:
I know that Marlin is slightly slower than ductus for shape size ~ 20:
Ductus seems using 16x16 blocks whereas Marlin uses 32x32 tiles so the
new RLE approach is not in use (raw encoding) i.e. lots of zero-fill /
array copy operations.

What makes you think that Ductus is using 16x16 blocks? It is designed around 32x32 tiles to the point where it isn't really configurable (they were modeling what they thought was going to be AA rendering hardware that would take over the world in the mid-90s and it never happened). We did try different block sizes when we were integrating the technology, back when they were still designing the hardware chips, but once we settled on 32x32 they baked it in like it was hardware.

Finally Marlin has still a disavantage: it does not perform shape
clipping in contrary to ductus = it's a remaining TODO.

    I'm code reading now:

    ArrayCache.java, line 205 - should that be needSize there?  Also,
    should these tests be > or >=?

I wanted to limit the size to 2M (Integer.MAX_VALUE) but it wanted 2
passes: first, return 2M, then if more needed, fail !
If prefer using >= to be simpler ie always check the current size.

The point is that the hard failure is a condition of when we need more than we can provide, not when we "already have" more than we can provide. needSize should cause the hard failure, not the current size. And if needSize is going to cause the hard failure then why use >= instead of > - we can supply MAX_INT if that is what is "needed".

Also, there is a potential bug right now that curSize could be < MAX_INT, but needSize could be > MAX_INT, but the test at line 205 and the adjustment at line 212 will decide that we can return with MAX_INT being the answer - even though that is less than what we really need.

It is just much more straightforward to have the tests be:

- if we are planning to allocate more than MAX
   (that isn't going to work, so...)
   - then if we really *need* more than MAX => exception
   - otherwise just allocate MAX

Another way to code this would be to first clip against MAX as being a hard limit and then throw an exception if the planned amount "isn't enough":

if (size > MAX_INT) size = MAX_INT;
if (size < needSize) exception("We couldn't allocate enough");
return size;

    Renderer.java, line 1446 - initial value of useBlkFlags will be
    false if heuristics is enabled
    Renderer.java, line 1329 - we send a row to the cache before we
    update heuristics at line 1332
    - taken together that means that with heuristics the very first
    cache line sent out will always be non-RLE?


You're right:
In my first tests, I was testing every scanline but it was costly (8x
more tests but always clean blkFlags when used).
Now I only check 1/8th scanlines ie once per pixel row as it is roughly
"continuous" (maybe false positive) and it implies the very first row is
always non RLE encoded.

Is this intentional? Or can the heuristic be done at the beginning so that the first pixel row also benefits from it?

    Once enableLogs is on, isUseLogging controls whether it goes to a
    log file or just prints to System.out.

    Also, there are other static booleans turn on and off different
    pieces of the logging at a finer granularity.  The only logging that
    was happening without being wrapped by a "if (doFooLogging)"
    conditional were the logInfos that executed when the RE was
    initialized.  I suppose those could have been neutered with a
    different "doStartupInfo" boolean, but I figured I would turn them
    all off by default for now.

    We can enable runtime-logging later on if we find a decent way to
    have it be low impact when not runtime-enabled...


(I deliberately disabled doMonitors and doChecks (for performance reasons))


However I would prefer having enableLogs = true if the renderer is in
verbose mode: "  -Dsun.java2d.renderer.verbose=true".

So it would display the startup informations in this case (including
tuning settings ...) and it would allow gathering Marlin statistics (if
doStats = true).

I could see that, but perhaps it should be controlled by a marlin-specific attribute?

In any case, this is very simple to add later once we get this integrated. I just wanted to be sure that initial integration took the path of least resistance to silencing it.

Moreover, I should refactor j.u.l.Logger usage by PlatformLogger (TODO)
and we should rename all Marlin System properties to use the prefix
"sun.java2d.marlin..."

I would support that for marlin-specific properties...

                ...jim

Reply via email to