1808756 - Reduce code duplication in batch.rs

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, task, P3)

Description

[Nicolas Silva [:nical]]

batch.rs contains a pretty large amount of code duplication (see the 2k+ lines add_prim_to_bacth), to the point that making some changes is tedious/error-prone and adding new types of primitives is discouraging.

The structure of a big chunk of the duplicated code looks like this:

match type {
    PrimType::Foo(foo) => {
        // resolve handles
        // compute belnd mode 
        // make batch key
        // iterate tiles or segments
    }
    PrimType::Bar(bar) => {
        // mostly the same as the above with small differences
    }
    // etc.
}

Primitives that use the image brush shader

There are multiple primitives that have almost the exact same code in the big match statement and do exactly the same thing: resolve a handle to some image data which we'll render with the brush shader. The image primitive is one example but all primitives that render their pattern in an image are in the same boat (all cached gradients for example).

For these it would be great to to desugar into an image primitive before batching to avoid 100-ish lines of code that get duplicated.

Not having to deal with these during batching would also make it easier to add new types of pre-rendered patterns.

Other primitive types

More generally the flow remains mostly the same, so it would be great to rework the code so that in the end it looks more like:

let blend_mode = ...
let batch_key = // ...
for handle in prim.handle_to_resolves {
    handles.push(render_tasks.resolve(handle));
});

match type {
    PrimType::Foo(foo) => {
        // very few specific things
    }
    PrimType::Bar(bar) => {
        // very few specific things
    }
}

if prim.segments.is_empty() {
    add_prim_to_batch(...)
} else {
    for segment in &prim.segments {
        // etc.
        add_prim_to_batch(...)
    }
}

Comment 1

[Nicolas Silva [:nical]]

Attachment: Bug 1808756 - Factor out some repetitive code around the setup of primitive headers. r=#gfx-reviewers

Comment 2

[Nicolas Silva [:nical]]

Attachment: Bug 1808756 - Reduce duplicated calls to add_brush_instance_to_batches in picture batching code. r=#gfx-reviewers

Comment 3

[Nicolas Silva [:nical]]

Attachment: Bug 1808756 - Reduce duplicated calls to PrimitiveHeaders::push in the picture batching code. r=#gfx-reviewers

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/532f34fffb9d
https://hg.mozilla.org/integration/autoland/rev/7e7f652905fc
Reduce duplicated calls to add_brush_instance_to_batches in picture batching code. r=gfx-reviewers,lsalzman
https://github.com/mozilla-firefox/firefox/commit/270a3076718a
https://hg.mozilla.org/integration/autoland/rev/fba813b71f18
Reduce duplicated calls to PrimitiveHeaders::push in the picture batching code. r=gfx-reviewers,lsalzman
https://github.com/mozilla-firefox/firefox/commit/b58eba3b57bd
https://hg.mozilla.org/integration/autoland/rev/59d3d68fff17
Factor out some repetitive code around the setup of primitive headers. r=gfx-reviewers,lsalzman

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/7e7f652905fc
https://hg.mozilla.org/mozilla-central/rev/fba813b71f18
https://hg.mozilla.org/mozilla-central/rev/59d3d68fff17