<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
Hi Dani,<br>
<br>
Welcome, and thank you for taking a look at this problem.<br>
<br>
I don't know enough yet to suggest the best course of action, but I
took a quick look at it. I only see the one call to <span>the
native RenderingQueue::</span>flush -- from freeSpace(int) if
autoFlush is true. <span>RenderingQueue::</span>flush does a Java
upcall to WCRenderQueue::fwkFlush which then
calls WCRenderQueue::flush to decode and process the buffer. The
native <span>RenderingQueue::</span>flushBuffer method does a Java
upcall to WCRenderQueue::fwkAddBuffer which will add the buffer to a
linked list of buffers, and then allocate a new BufferData wrapper
object. It then calls WCRenderQueue::flush if the total size of the
linked list of buffers it too large.<br>
<br>
One thing that should be checked is whether the call to
WCRenderQueue::flush from native <span>RenderingQueue::</span>flushBuffers
and then again from <span>RenderingQueue::</span>flush is causing
the problem. Instrumenting the code on the Java side (which is
usually easier if it gives you the information you need), might show
what's going on.<br>
<br>
-- Kevin<br>
<br>
<br>
<div class="moz-cite-prefix">On 10/20/2024 7:01 AM, dani-kurmann
wrote:<br>
</div>
<blockquote type="cite" cite="mid:T7cXH4DPL2NonS-5EdEvn0V7Wdu1YJZ_gvfPZg5ImgLVZfVNBGj7DCDzTOKe5wphkh-tl_ZLKUZVR3dTMBs9lxSY2kI3LNorqZOrmmDNRy4=@protonmail.com">
<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" moz-do-not-send="true" class="moz-txt-link-freetext">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" moz-do-not-send="true" class="moz-txt-link-freetext">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" moz-do-not-send="true">Proton Mail</a> secure email. </div>
</div>
</blockquote>
<br>
</body>
</html>