987523 - Vsync-triggered CompositorParent

Status: RESOLVED INVALID

(Core :: Graphics: Layers, defect, P2)

Description

[Vincent Lin[:vilin]]

A part of Project Butter on B2G. To make CompositorParent being triggered by vsync notification. It may need bug 980321 to make layer update async.

Comment 1

[Vincent Lin[:vilin]]

Attachment: bug-987523-fix.patch

WIP

Comment 2

[Vincent Lin[:vilin]]

Attachment: bug-987523-fix.patch

WIP

Comment 3

[Vincent Lin[:vilin]]

Attachment: bug-987523-fix.patch

WIP~
Register with info. of observer type.

Comment 4

[u459114]

Comment on attachment 8405955 [details] [diff] [review]
bug-987523-fix.patch

Review of attachment 8405955 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/ipc/CompositorParent.cpp
@@ +95,5 @@
> +// Guarantee in Main thread
> +static void CompositorRegisterVsyncObserver(CompositorParent* aCompositorParent)
> +{
> +  if (sCompositorParent != aCompositorParent) {
> +    RegisterVsyncObserver(aCompositorParent, VSYNC_TYPE_COMPOSITE);

VsyncDispatcher::RegistCompositer(this)

@@ +104,5 @@
> +// Guarantee in Main thread
> +static void CompositorUnregisterVsyncObserver(CompositorParent* aCompositorParent)
> +{
> +  if (aCompositorParent != nullptr) {
> +    UnregisterVsyncObserver(aCompositorParent, VSYNC_TYPE_COMPOSITE);

VsyncDispatcher::UnregistCompositer(this)

@@ +124,5 @@
> +    if (isVsyncObserver)
> +      sMainLoop->PostTask(FROM_HERE, NewRunnableFunction(&CompositorRegisterVsyncObserver, aCompositorParent));
> +    else
> +      sMainLoop->PostTask(FROM_HERE, NewRunnableFunction(&CompositorUnregisterVsyncObserver, aCompositorParent));
> +  }

I will make sure these two functions are thread safe
VsyncDispatcher::Un/RegistCompositer

You don't need to handle thread issue here

@@ +553,5 @@
> +  // Don't schedule mCurrentCompositeTask later. Just start observing Vsync.
> +  if (Preferences::GetBool("hal.hw-vsync", false)) {
> +    CompositorIsVsyncObserver(this, true); 
> +    return;
> +  }

Read this pref in constructor and keep it in a data member

@@ +619,5 @@
>    TimeDuration minFrameDelta = TimeDuration::FromMilliseconds(
>      rate == 0 ? 0.0 : std::max(0.0, 1000.0 / rate));
>  
> +  if (!Preferences::GetBool("hal.hw-vsync", false))
> +    mCurrentCompositeTask = NewRunnableMethod(this, &CompositorParent::Composite);

https://bugzilla.mozilla.org/show_bug.cgi?id=980241#c33

The same. You don't need to care about we use SW or HW vsync beneath. The only thing that we need to care here is whether SILK enable

Comment 5

[Vincent Lin[:vilin]]

Attachment: bug-987523-fix.patch

Include "gfx.silk-enable" preference.

Comment 6

[Vincent Lin[:vilin]]

Attachment: bug-987523-fix.patch

Include "gfx.silk-enable" preference.

Comment 7

[Vincent Lin[:vilin]]

Attachment: bug-987523-fix.patch

Include "gfx.silk-enable" preference.

Comment 8

[Andreas Gal :gal]

I would prefer not using code names in the code. Use Vsync or some such instead please.

Comment 9

[u459114]

https://bugzilla.mozilla.org/show_bug.cgi?id=980241#c37

The same reason, close this one