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.h...@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....@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....@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.rushfo...@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.h...@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.h...@oracle.com> <david.h...@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.h...@oracle.com> <david.h...@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) > >
