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

Status: RESOLVED FIXED

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

Description

[Kelsey Gilbert [:jgilbert]]

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

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1404196 - Add profiling labels for WebGL draw commands. -

Review commit: https://reviewboard.mozilla.org/r/184922/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184922/

Comment 2

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8913548 [details]
Bug 1404196 - Add profiling labels for WebGL draw commands. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184922/diff/1-2/

Comment 3

[Michael Leu[:Lenzak](UTC+8)]

This patch breaks quite a few conformance tests, I think we should only performance this validation when we use ANGLE_instanced_array extension.

Comment 4

[Kelsey Gilbert [:jgilbert]]

Let me finish debugging it.

Comment 5

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8913548 [details]
Bug 1404196 - Add profiling labels for WebGL draw commands. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184922/diff/2-3/

Comment 6

[Kelsey Gilbert [:jgilbert]]

Looking better: (linux came back green. win/mac TBD)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fae36bb74a5cc75673fe13b48e38f469a2964d85

Comment 7

[Michael Leu[:Lenzak](UTC+8)]

Comment on attachment 8913548 [details]
Bug 1404196 - Add profiling labels for WebGL draw commands. -

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

LGTM

Comment 8

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1404196 - Add CacheMap for simplifying complex cache dependency invalidation. -

Review commit: https://reviewboard.mozilla.org/r/194786/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/194786/

Comment 9

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1404196 - Simplify and repair vertex fetch. -

Review commit: https://reviewboard.mozilla.org/r/194788/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/194788/

Comment 10

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8913548 [details]
Bug 1404196 - Add profiling labels for WebGL draw commands. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184922/diff/3-4/

Comment 11

[Code Review Bot [:reviewbot]]

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

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8913548 [details]
Bug 1404196 - Add profiling labels for WebGL draw commands. -

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

Comment 13

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8923665 [details]
Bug 1404196 - Add CacheMap for simplifying complex cache dependency invalidation. -

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

lgtm!

Comment 14

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8923666 [details]
Bug 1404196 - Simplify and repair vertex fetch. -

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

lgtm. thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
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.

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/f04beba33601
https://hg.mozilla.org/mozilla-central/rev/3cd543f596da
https://hg.mozilla.org/mozilla-central/rev/a8d61165ea3e
https://hg.mozilla.org/mozilla-central/rev/f9baf7fe5e81