Closed Bug 1308057 Opened 7 years ago Closed 7 years ago

Merge WebGLTimerQuery into WebGLQuery

Categories

(Core :: Graphics: CanvasWebGL, defect, P1)

Product:
Core
Component:
Graphics: CanvasWebGL
Version:
50 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox51 --- fixed
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: jgilbert, Assigned: jgilbert)

References

(
URL
)

Details

(Keywords: dev-doc-complete, Whiteboard: gfx-noted)

Attachments

(1 file, 11 obsolete files)

all patches in this bug
39 bytes, text/plain
gchang
: approval-mozilla-aurora+
gchang
: approval-mozilla-beta+
Details 
      No description provided.
Comment on attachment 8798265 [details]
Bug 1308057 - retvals should start null, and missing return. -

https://reviewboard.mozilla.org/r/83774/#review82346
Attachment #8798265 - Flags: review?(ethlin) → review+
Comment on attachment 8798266 [details]
Bug 1308057 - Merge WebGLTimerQuery into WebGLQuery. -

https://reviewboard.mozilla.org/r/83776/#review82408

::: dom/canvas/WebGL2ContextQueries.cpp:134
(Diff revision 1)
> -        return;
> +        return ErrorInvalidOperation("%s: Query target already active.", funcName);
>  
> -    WebGLRefPtr<WebGLQuery>& querySlot = GetQuerySlotByTarget(target);
> +    ////
> -    WebGLQuery* activeQuery = querySlot.get();
>  
> -    if (!activeQuery || target != activeQuery->mType)
> +    if (!query->BeginQuery(target))

I think you should pass 'funcName' to BeginQuery() and remove the 'funcName' in WebGLQuery::BeginQuery.
Comment on attachment 8798266 [details]
Bug 1308057 - Merge WebGLTimerQuery into WebGLQuery. -

https://reviewboard.mozilla.org/r/83776/#review82414
Attachment #8798266 - Flags: review?(ethlin) → review+
Comment on attachment 8809630 [details]
Bug 1308057 - WebGL objects should not be no-interface. -

https://reviewboard.mozilla.org/r/92166/#review92178
Attachment #8809630 - Flags: review?(kyle) → review+
Comment on attachment 8809628 [details]
Bug 1308057 - EXT_disjoint_timer_query is no longer Draft. -

https://reviewboard.mozilla.org/r/92162/#review92560
Attachment #8809628 - Flags: review?(ethlin) → review+
Comment on attachment 8809631 [details]
Bug 1308057 - Fix errors. -

https://reviewboard.mozilla.org/r/92168/#review92562
Attachment #8809631 - Flags: review?(ethlin) → review+
Comment on attachment 8809669 [details]
Bug 1308057 - GPU_DISJOINT shouldn't assert the ext. -

https://reviewboard.mozilla.org/r/92210/#review92564
Attachment #8809669 - Flags: review?(ethlin) → review+
Comment on attachment 8809633 [details]
Bug 1308057 - Use pointer-to-query-slot for is-active status. DeleteQuery should clear its slot if it has one. -

https://reviewboard.mozilla.org/r/92172/#review92566
Attachment #8809633 - Flags: review?(ethlin) → review+
Comment on attachment 8809632 [details]
Bug 1308057 - GetQuery(CURRENT_QUERY) should return null for target mismatch, for targets that share the same slot. -

https://reviewboard.mozilla.org/r/92170/#review92568
Attachment #8809632 - Flags: review?(ethlin) → review+
Comment on attachment 8809629 [details]
Bug 1308057 - Update tests. -

https://reviewboard.mozilla.org/r/92164/#review92570
Attachment #8809629 - Flags: review?(ethlin) → review+
Comment on attachment 8798266 [details]
Bug 1308057 - Merge WebGLTimerQuery into WebGLQuery. -

https://reviewboard.mozilla.org/r/83776/#review82408

> I think you should pass 'funcName' to BeginQuery() and remove the 'funcName' in WebGLQuery::BeginQuery.

If it's not there with the recent updates, we can look at doing this in a later cleanup.
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bba645e82e8
Convert WebGLExtensionDisjointTimerQuery.cpp to 4-space.
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8992921aab5
retvals should start null, and missing return. - r=ethlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d8ce263df30
Merge WebGLTimerQuery into WebGLQuery. - r=ethlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/356190415401
Update bindings. - r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4e073f1cb11
EXT_disjoint_timer_query is no longer Draft. - r=ethlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c047b0aadea
Update tests. - r=ethlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b8f6b2d3982
WebGL objects should not be no-interface. - r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/562012ee3497
Fix errors. - r=ethlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4437f996855
GetQuery(CURRENT_QUERY) should return null for target mismatch, for targets that share the same slot. - r=ethlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/16ab7b5f3a08
Use pointer-to-query-slot for is-active status. DeleteQuery should clear its slot if it has one. - r=ethlin
https://hg.mozilla.org/integration/mozilla-inbound/rev/216accfaadc2
GPU_DISJOINT is false unless we have the ext. - r=ethlin
Attachment #8798265 - Attachment is obsolete: true
Attachment #8798266 - Attachment is obsolete: true
Attachment #8798267 - Attachment is obsolete: true
Attachment #8809628 - Attachment is obsolete: true
Attachment #8809629 - Attachment is obsolete: true
Attachment #8809630 - Attachment is obsolete: true
Attachment #8809631 - Attachment is obsolete: true
Attachment #8809632 - Attachment is obsolete: true
Attachment #8809633 - Attachment is obsolete: true
Attachment #8809669 - Attachment is obsolete: true
Comment on attachment 8798264 [details]
Bug 1308057 - Add to interface list. -

https://reviewboard.mozilla.org/r/83772/#review92848
Attachment #8798264 - Flags: review?(kyle) → review+
Approval Request Comment
[Feature/Bug causing the regression]: webgl2
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:
Attachment #8798264 - Attachment is obsolete: true
Attachment #8818744 - Flags: approval-mozilla-beta?
Attachment #8818744 - Flags: approval-mozilla-aurora?
Comment on attachment 8818744 [details]
all patches in this bug

WebGL2 related patch. Beta51+ & Aurora52+. Should be in 51 beta 8.
Attachment #8818744 - Flags: approval-mozilla-beta?
Attachment #8818744 - Flags: approval-mozilla-beta+
Attachment #8818744 - Flags: approval-mozilla-aurora?
Attachment #8818744 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.