<div style="font-family: Arial, sans-serif; font-size: 14px;"><span>Hi everyone, nice to be here.</span><div><br></div><div><span>I don't want to be overzealous, but I did sign up for a bugfix :)</span></div><div><span>So here is me, socializing:</span></div><div><br></div><div><span>I ran into the bug in a similiar scenario as described in the Bug report: <a target="_blank" rel="noreferrer nofollow noopener" href="https://bugs.openjdk.org/browse/JDK-8229902">https://bugs.openjdk.org/browse/JDK-8229902</a></span></div><div><br></div><div><span>It's a buffer-problem. The culprit is the flush()-call on line 64 of /modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/RenderingQueue.cpp (in the freeSpace(int size) function)</span></div><div><br></div><div><span> if (m_buffer && !m_buffer->hasFreeSpace(size)) {</span></div><div><span> flushBuffer();</span></div><div><span> </span></div><div><span> if (m_autoFlush) { </span></div><div><span> flush();</span></div><div><span> }</span></div><div><span> </span></div><div><span> }</span></div><div><br></div><div><span>this gets called, before a shape(or possibly something else) is added to the bytebuffer. If there is not enough space left in the buffer, here we go:</span></div><div><br></div><div><span>The flushBuffer()-Call works fine. It sets up new Buffer Space as intended and, I guess, flushes the buffer.</span></div><div><span>The flush()-call however leads to the described Bug. Just comment it out to test it. But thats not a bugfix, of course.</span></div><div><br></div><div><span>Its very interesting where the two calls lead, this is right in the sweet spot between java and c++ and openjdk and webkit. I dont claim to understand half of it. But I do find it very interesting. </span></div><div><br></div><div><span>This is the point where I ask for help: I can propose a bugfix, but I want to understand the issue deeper. Maybe this is a newbie-problem, but I have problems understanding the above mentioned sweet spot. So any advice is very welcome!</span></div><div><br></div><div><span>Please DO correct me, and show me that other approach that I dont see, but the way I see it, there's two ways to approach this thing:</span></div><div><span>A) Treat it as an initialisation-problem. e.g. There is nothing wrong with RenderingQueue, it just needs to be initialised with m_autoFlush=false. (in this case, at least)</span></div><div><span>B) There seems to be something strange going on in RenderingQueue, understand it and fix it.</span></div><div><br></div><div><br></div><div><span>A) is a lot easier: just follow back to where the RenderingQueue Constructor gets called: /modules/javafx.web/src/main/native/Source/WebCore/platform/graphics/java/ImageBufferJavaBackend.cpp, line 82:</span></div><div><br></div><div><span>auto context = makeUnique<GraphicsContextJava>(new PlatformContextJava(wcRenderQueue, true));</span></div><div><br></div><div><span>The second Argument for the PlatformContext-constructor sets the m_autoFlush-Variable upon which the flush()-call in the freeSpace-function in RenderingQueue.cpp is conditional. Set it to false.</span></div><div><span>as a git commit: <a target="_blank" rel="noreferrer nofollow noopener" href="https://github.com/CodeMonkeyIsland/jfx/commit/96f23307c7b6f324bac416b90c0eac4ad40b13a8">https://github.com/CodeMonkeyIsland/jfx/commit/96f23307c7b6f324bac416b90c0eac4ad40b13a8</a></span></div><div><br></div><div><span>pro: </span></div><div><span>-simple fix </span></div><div><span> -no need to mess around in more than one file</span></div><div><span> -looks like this is a normal way to use RenderingQueue</span></div><div><span>con: </span></div><div><span> -more and more feels like a hack</span></div><div><br></div><div><br></div><div><span>B) is can of worms: You can comment out not only the flush()-call in the freespace-function, but the flush() function itself. At first, I thought, that the flush()-function is still needed for a flush-call during flushBuffer() outside RenderingQueue. But that was not the same flush()-function. I just tested that, and it works. (havent checked yet, if this breaks something else)</span></div><div><span>So assuming, the flush()-function is really defunct and not needed(and I would have to dive into this a lot deeper to make that statement as a fact), this is a chance to do some spring-cleaning in RenderingQueue. Again, I only see two main ways of approaching this:</span></div><div><span>- make m_autoFlush obsolete. Since (if the assumption above is right) it already flushes regardless of how m_autoFlush is set, lets stop pretending. Remove m_autoFlush, adjust the constructors and the constructor calls outside RenderingQueue.</span></div><div><span>- make m_autoFlush great again :) It would basically mean separating flushBuffer() into two functions, one that does the addBuffer-part and one that does the flushing. Then do the addBuffer unconditional and the flushing conditional on m_autoFlush().</span></div><div><span>The "problem" here being, that other parts of the codebase seem to have gone the way of approach A). So another piece of code initializing RenderingQueue with m_autoFlush=false doesnt mean that they dont want to autoFlush. So every call of the constructor has to be analysed, if the intention is to autoflush or not. I dont see the use-case for not flushing. So this approach might very well lead to a situation, where every single constructor call sets m_autoFlush to true and m_autoFlush is kind of obsolete again. Or not. Anyone has an example, where it shouldnt flush? Maybe there is runtime to be optimised by planning when to flush? I dont know...</span></div><div><br></div><div><span>pro: </span></div><div><span>-if there is a problem in RenderingQueue, then thats the place to fix it! No matter the consequences.</span></div><div><span>con: </span></div><div><span>-even if its an easy fix in RenderingQueue, it will change its behaviour. Or at the very least make autoFlush obsolete. This has consequences.</span></div><div><br></div><div><br></div><div><span>Approach B) seems a lot scarier than A), but if it must be done, it should be done. If you guys think this is the rabbit hole to be, I would be willing to go there. However, I don't feel competent to overview all the consequences of this. I could really need some advice and help on that road, I guess thats why Im here.</span></div><div><span>I could imagine doing A) as a temporary fix and an exercise. Then exploring B) deeper with some help.</span></div><div><br></div><div><span>As for testing: At first glance, this seems like a non-automated Mk.1 eyeball-test. On a closer look however, you could do something like the Bug-Class in the bug-report, extract the RenderingContext-imagedata, pass it from js to java and check the color of the pixel, to see if a shape has been drawn. A window would have to be opened for this test, I dont know if thats allowed. (But why not, I guess)</span></div><div><br></div><div><br></div><div><span>So what do you guys think? Does that sound like a plan? Does this break something, that I don't see?</span></div><div><span>I am new here. Anyone willing to have a look into this and maybe point me in the right direction sometimes?</span></div><div><br></div><div><br></div><div><span>kind regards</span></div><div><br></div><div><span>Dani Kurmann</span></div><div><br></div><div><br></div><div><br></div><div><span>p.s. the Bug class in the bug report does not work anymore.(at least for me, forked or unforked) the script in executeScript doesn't get executed. As a quick fix, you can pass the function in the html document like so:</span></div><div><br></div><div><span> WebEngine engine = webView.getEngine();</span></div><div><span> String content="<html><body><canvas id=\"canvas\" width=\"600\" height=\"600\" style=\"border: 1px solid red;\"></canvas><script>window.onload = function() {";</span></div><div><span> content=content</span></div><div><span> + " var canvas = document.getElementById('canvas');"</span></div><div><span> + " var c = canvas.getContext('2d');"</span></div><div><span> + " var size = 4;"</span></div><div><span> + " var step = size + 2;"</span></div><div><span> + " for (var y = 0; y <= canvas.height; y = y + step) {"</span></div><div><span> + " for (var x = 0; x <= canvas.width; x = x + step) {"</span></div><div><span> + " c.fillRect(x, y, size, size);"</span></div><div><span> + " }"</span></div><div><span> + " }";</span></div><div><span> content=content+"}</script></body></html>";</span></div><span> engine.loadContent(content);</span><br></div><div style="font-family: Arial, sans-serif; font-size: 14px;"><br></div>
<div class="protonmail_signature_block" style="font-family: Arial, sans-serif; font-size: 14px;">
<div class="protonmail_signature_block-user protonmail_signature_block-empty">
</div>
<div class="protonmail_signature_block-proton">
Sent with <a target="_blank" href="https://proton.me/mail/home">Proton Mail</a> secure email.
</div>
</div>