Closed Bug 987523 Opened 6 years ago Closed 5 years ago

Vsync-triggered CompositorParent

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED INVALID

People

(Reporter: vlin, Assigned: jerry)

References

Details

(Keywords: perf, Whiteboard: [c=handeye p= s=2014.06.20.t u=])

Attachments

(1 file, 5 obsolete files)

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.
Blocks: 980321
No longer depends on: 980321
No longer blocks: 980321
Depends on: 980321
Blocks: Silk
Component: Performance → Graphics: Layers
Product: Firefox OS → Core
Keywords: perf
Priority: -- → P2
Whiteboard: [c=handeye p= s= u=]
Depends on: 987529
Attached patch bug-987523-fix.patch (obsolete) — Splinter Review
WIP
Depends on: 987527
Attached patch bug-987523-fix.patch (obsolete) — Splinter Review
WIP
Attachment #8398390 - Attachment is obsolete: true
No longer depends on: 987529
Attached patch bug-987523-fix.patch (obsolete) — Splinter Review
WIP~
Register with info. of observer type.
Attachment #8402433 - Attachment is obsolete: true
Attachment #8405955 - Flags: feedback?(slee)
Attachment #8405955 - Flags: feedback?(cku)
Status: NEW → ASSIGNED
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
Attachment #8405955 - Flags: feedback?(cku)
Attached patch bug-987523-fix.patch (obsolete) — Splinter Review
Include "gfx.silk-enable" preference.
Attachment #8405955 - Attachment is obsolete: true
Attachment #8405955 - Flags: feedback?(slee)
Attached patch bug-987523-fix.patch (obsolete) — Splinter Review
Include "gfx.silk-enable" preference.
Attachment #8418505 - Attachment is obsolete: true
Include "gfx.silk-enable" preference.
Attachment #8418506 - Attachment is obsolete: true
I would prefer not using code names in the code. Use Vsync or some such instead please.
Assignee: vlin → hshih
https://bugzilla.mozilla.org/show_bug.cgi?id=980241#c37

The same reason, close this one
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → INVALID
Whiteboard: [c=handeye p= s= u=] → [c=handeye p= s=2014.06.20.t u=]
You need to log in before you can comment on or make changes to this bug.