[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: Pausing Quantum Renderer
From:       Johan Vos <johan.vos () gluonhq ! com>
Date:       2015-11-23 20:27:11
Message-ID: CABxFH2EkLoYE2kYey83oRiyCDP2_AjFLCjx1EDU076vxVakE-A () mail ! gmail ! com
[Download RAW message or body]

The reason I have it in the try is because we need the finally block.

In the postPulse() method, setPulseRunning() is called before the pulse()
method will be called.
Hence, if we want a null pulse we still have to call endPulseRunning()
(which is in the finally block) or we will never have setPulseRunning()
returning false anymore.

- Johan

On Mon, Nov 23, 2015 at 5:41 PM, David Hill <David.Hill@oracle.com> wrote:

> On 11/22/15, 6:24 AM, Johan Vos wrote:
>
> I implemented this in the javafxports clone of the OpenJFX 8u-dev repo,
> and the diff is here:
>
>
> https://bitbucket.org/javafxports/8u-dev-rt/commits/67a0fded8208095bd04efda6045aa257e245d6bc
>
> I am more than happy to create an issue in the openjdk bug system
> (enhancement?) and provide a patch there as well, but I think it needs a
> bit more discussion first?
>
>
> My only slight concern looking at the patch is where we bail out of
> pulse(). At the moment you have between:
>     PulseLogger.pulseStart();
> and because of the finally block,
>     PulseLogger.pulseEnd();
>
> my first thought was that the return should be before the try block as
> this is a "null" pulse.
>
> I think I am fine with it either way though.
>
> Please file a bug on this (does not need to be an enhancement).
>
> Dave
>
>
> - Johan
>
> On Sat, Nov 21, 2015 at 9:23 PM, Johan Vos <johan.vos@gluonhq.com> wrote:
>
>> I have a working implementation that needs more stress-testing on
>> different platforms, but it seems a good and easy solution so far.
>> I have this on QuantumToolkit:
>>
>>     @Override
>>     public void pauseRenderer(){
>>         Application.invokeAndWait(() -> this.pause = true);
>>         PaintCollector.getInstance().waitForRenderingToComplete();
>>     };
>>
>>     public void resumeRenderer(){
>>         Application.invokeAndWait(() -> this.pause = false);
>>     };
>>
>> pause is a boolean that is checked for in
>> void pulse(boolean collect) { ... }
>>
>> The difficulty I mentioned in my previous mail (how do we know there are
>> no renderJobs pending/running) was solved easily as there exists this
>> PaintCollector.waitForRenderingToComplete method.
>> This might make the pauseRenderer a bit slower, and maybe this is not
>> needed in all usecases. In that case, we can remove it from the
>> pauseRenderer() and we can add it either in the Monocle implementation that
>> will call pauseRenderer, or in a Android/iOS specific code.
>>
>> However, it seems to me that if you want to pause the renderer, you most
>> often want to make sure no one is still writing to the glSurface after the
>> pauseRenderer method is called, so I think it makes sense to keep it there?
>>
>> - Johan
>>
>>
>>
>>
>>
>>
>> On Fri, Nov 20, 2015 at 8:14 AM, Johan Vos <johan.vos@gluonhq.com> wrote:
>>
>>> I didn't plan to intercept or cancel pending/submitted jobs, but I have
>>> to wait until they are done before the android callback method returns.
>>>
>>> When Android is about to destroy the context, it will call the
>>> surfaceTextureDestroyed method on the Activity (the FXActivity in our
>>> case). As long as that method doesn't return, the context won't be
>>> destroyed. But once that method returns, the context might become invalid
>>> any moment. So if there are still jobs that want to do a swapBuffer after
>>> we return, those can crash or (even worse) corrupt the egl system.
>>>
>>> So it seems to me inside the implementation of surfaceTextureDestroyed,
>>> we need to achieve 2 things:
>>> 1. make sure no new pulses are generated.
>>> 2. don't return while the QuantumRenderer is still executing jobs.
>>>
>>> Those 2 things can be combined in a single Toolkit.pauseRenderer() but
>>> it might be better to only achieve the first task in
>>> Toolkit.pauseRenderer().
>>> The contract for this method is than that no new pulses will be
>>> generated, but existing renderJobs might still be running when this method
>>> returns.
>>> The second thing, waiting for the renderJobs to be finished, can be done
>>> in the Android specific implementation.
>>>
>>> - Johan
>>>
>>>
>>>
>>> On Thu, Nov 19, 2015 at 10:20 PM, Kevin Rushforth <
>>> kevin.rushforth@oracle.com> wrote:
>>>
>>>> This might be a tricky semantic to achieve, and great care will be
>>>> needed to ensure no deadlocks or race conditions. Not running any more
>>>> pulses after this method returns seems fine, but it might be better to let
>>>> any existing renderJobs run (possibly discarding the results). This way you
>>>> could send the pause message to the renderer as a special renderJob and not
>>>> have to worry about jobs that are scheduled but not yet run.
>>>>
>>>> -- Kevin
>>>>
>>>>
>>>>
>>>> Johan Vos wrote:
>>>>
>>>>> After some experiments, here is my current thinking:
>>>>>
>>>>> Toolkit can have 2 new methods:
>>>>> pauseRenderer()
>>>>> resumeRenderer()
>>>>>
>>>>> When pauseRenderer is called, it should be guaranteed that after this
>>>>> call,
>>>>> no new pulses are fired until resumeRenderer is called.
>>>>> That is not hard, but it is not enough. Before we pause the pulses, the
>>>>> previous pulse probably submitted a renderJob to Prism, executed on the
>>>>> QuantumRenderer ThreadPoolExecutor. That job should run fine, as the
>>>>> next
>>>>> pulse (when we're back) will call
>>>>> GlassScene.waitForRenderingToComplete().
>>>>> So we have to wait until there are no running or pending tasks in the
>>>>> QuantumRenderer as well.
>>>>>
>>>>> - Johan
>>>>>
>>>>>
>>>>> On Wed, Nov 18, 2015 at 9:58 PM, David Hill <David.Hill@oracle.com>
>>>>> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> On 11/18/15, 3:45 PM, Johan Vos wrote:
>>>>>>
>>>>>> Johan,
>>>>>>     I think that it would be reasonable to put in something to Quantum
>>>>>> that causes the render loop to "pause", and then resume later. I
>>>>>> envision
>>>>>> this toggle as causing the render loop to skip, rather than tinkering
>>>>>> with
>>>>>> the pulses.
>>>>>>
>>>>>> When resume is called, it might be best to treat the world as dirty.
>>>>>>
>>>>>> Added to Toolkit, this would allow someone like Monocle to make the
>>>>>> toggles as is appropriate.
>>>>>>
>>>>>> If this works for you, perhaps you could prototype it ?
>>>>>>
>>>>>> regards,
>>>>>>    Dave
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Android, a JavaFX Application might transfer control to another
>>>>>>> app
>>>>>>> (and
>>>>>>> become invisible) and enter the foreground later. In that case, the
>>>>>>> GLSurface we are rendering on becomes invalid. In order to avoid
>>>>>>> problems
>>>>>>> and save battery, we want to pause the renderer thread, but this
>>>>>>> turns out
>>>>>>> to be more difficult than I expected.
>>>>>>>
>>>>>>> When our app transfers control, we get a callback from Android. We
>>>>>>> intercept this in javafxports, and we set the Screen width/height to
>>>>>>> 0/0
>>>>>>> as
>>>>>>> we don't want to render on the (invalid) surface while we lost
>>>>>>> control.
>>>>>>> When we regain control, we resize the Screen and the app renders
>>>>>>> again.
>>>>>>>
>>>>>>> The reason we set the width/height to 0/0 is because the
>>>>>>> PresentingPainter
>>>>>>> will call SceneState.isValid() and this returns false in case
>>>>>>> getWidth()
>>>>>>> or
>>>>>>> getHeight() are 0.
>>>>>>>
>>>>>>> However, SceneState extends PresentableState and it overrides the
>>>>>>> update
>>>>>>> method. It will call PresentatbleState.update() which will set the
>>>>>>> viewWidth to the width of the new Screen (hence, 0) , but after that
>>>>>>> it
>>>>>>> overwrites the viewWidth with camera.getViewWidth(). The latter still
>>>>>>> contains the old value. A quick inspection shows that
>>>>>>> camera.setViewWidth()
>>>>>>> is called when validate(...) is called on NGDefaultCamera, which is
>>>>>>> called
>>>>>>> by ES2Context.updateRenderTarget() which happens during rendering,
>>>>>>> hence
>>>>>>> *after* the PresentingPainter checks if the width is 0.
>>>>>>>
>>>>>>> So immediately after we set the width of the Screen to 0 (on the FX
>>>>>>> App
>>>>>>> Thread), a Pulse happens, and this one still things the screen is the
>>>>>>> original size. While the pulse is happening, the android system
>>>>>>> destroys
>>>>>>> our context, and the rendering fails. Moreover, the EGL system is in
>>>>>>> a
>>>>>>> unpredictable state (recreating the surface fails).
>>>>>>>
>>>>>>> A very dirty workaround for this is to wait for 1 pulse (with the new
>>>>>>> pulselistener API this should be possible) before we return from the
>>>>>>> callback method called by Android when the surface is about to be
>>>>>>> destroyed. That way, we will have 1 bogus rendering on an existing
>>>>>>> (but
>>>>>>> about-to-be-destroyed) surface.
>>>>>>>
>>>>>>> But it would be better if there is some way to tell the quantum
>>>>>>> renderer
>>>>>>> to
>>>>>>> immediately stop rendering. Existing pulses are no problem, as the
>>>>>>> renderLock guarantuees that we set the size to 0 only when no other
>>>>>>> thread
>>>>>>> (quantum renderer) has a lock on the renderLock.
>>>>>>>
>>>>>>> - Johan
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> --
>>>>>> David Hill<David.Hill@Oracle.com> <David.Hill@Oracle.com>
>>>>>> Java Embedded Development
>>>>>>
>>>>>> "A man's feet should be planted in his country, but his eyes should
>>>>>> survey
>>>>>> the world."
>>>>>> -- George Santayana (1863 - 1952)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>
>>
>
>
> --
> David Hill <David.Hill@Oracle.com> <David.Hill@Oracle.com>
> Java Embedded Development
>
> "A man's feet should be planted in his country, but his eyes should survey the world."
> -- George Santayana (1863 - 1952)
>
>
[prev in list] [next in list] [prev in thread] [next in thread]