Non-instanced draw calls must also check that at least one vertex attrib divisor is 0

RESOLVED FIXED in Firefox 58

Status

()

defect
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 wontfix, firefox58 fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(3 attachments)

Assignee

Description

2 years ago
Though it's a little unclear from the parent specs, this is explicitly stated in the WebGL 2 spec:
https://www.khronos.org/registry/webgl/specs/latest/2.0/#ENABLED_ATTRIBUTE
Assignee

Updated

2 years ago
Whiteboard: gfx-noted
Target Milestone: --- → mozilla58
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
This patch breaks quite a few conformance tests, I think we should only performance this validation when we use ANGLE_instanced_array extension.
Assignee

Comment 4

2 years ago
Let me finish debugging it.
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8913548 [details]
Bug 1404196 - Add profiling labels for WebGL draw commands. -

https://reviewboard.mozilla.org/r/184922/#review190480

LGTM
Attachment #8913548 - Flags: review?(cleu) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 11

2 years ago
mozreview-review
Comment on attachment 8923665 [details]
Bug 1404196 - Add CacheMap for simplifying complex cache dependency invalidation. -

https://reviewboard.mozilla.org/r/194786/#review199848


C/C++ static analysis found 1 defect in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: dom/canvas/CacheMap.cpp:14
(Diff revision 1)
> +namespace mozilla {
> +
> +void
> +CacheMapInvalidator::InvalidateCaches() const
> +{
> +    while (mCacheEntries.size()) {

Warning: The 'empty' method should be used to check for emptiness instead of 'size' [clang-tidy: readability-container-size-empty]

    while (mCacheEntries.size()) {
           ^
           !mCacheEntries.empty()

Comment 12

2 years ago
mozreview-review
Comment on attachment 8913548 [details]
Bug 1404196 - Add profiling labels for WebGL draw commands. -

https://reviewboard.mozilla.org/r/184922/#review199904
Attachment #8913548 - Flags: review?(dmu) → review+

Comment 13

2 years ago
mozreview-review
Comment on attachment 8923665 [details]
Bug 1404196 - Add CacheMap for simplifying complex cache dependency invalidation. -

https://reviewboard.mozilla.org/r/194786/#review199918

lgtm!
Attachment #8923665 - Flags: review?(dmu) → review+

Comment 14

2 years ago
mozreview-review
Comment on attachment 8923666 [details]
Bug 1404196 - Simplify and repair vertex fetch. -

https://reviewboard.mozilla.org/r/194788/#review200462

lgtm. thanks!
Attachment #8923666 - Flags: review?(dmu) → review+

Comment 15

2 years ago
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f04beba33601
Add profiling labels for WebGL draw commands. - r=daoshengmu
https://hg.mozilla.org/integration/mozilla-inbound/rev/3cd543f596da
Add CacheMap for simplifying complex cache dependency invalidation. - r=daoshengmu
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8d61165ea3e
Simplify and repair vertex fetch. - r=daoshengmu
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9baf7fe5e81
Update test failures.
Depends on: 1414524
Assignee

Updated

2 years ago
Blocks: 1414977
Assignee

Updated

2 years ago
Blocks: 1414524
No longer depends on: 1414524
Depends on: 1414996
Assignee

Updated

2 years ago
Blocks: 1414996
No longer depends on: 1414996
Assignee

Updated

2 years ago
Blocks: 1417312
You need to log in before you can comment on or make changes to this bug.