Commit 1b90388e authored by Jamie Nicol's avatar Jamie Nicol
Browse files

Bug 1731758 - Avoid Adreno 3xx driver bug in brush_blend shader. r=gfx-reviewers,nical

On some Adreno 3xx devices the brush blend shader renders
garbage. This appears to be due to declaring the v_funcs varying (used
for component transfer filters) as an ivec4, though it is unclear
exactly why this breaks it. Previously we used an int[4], but that ran
afoul of another driver bug on some Adreno 3xx devices (bug 1695912).

This patch alternatively packs each of the component functions in to 4
bits of an integer, and unpacks the values in the fragment
shader. However, due to yet another Adreno 3xx driver bug (bug
1630356) we cannot simply use an int, and instead must use the x
component of a vec2.

This fixes the brush_blend shader for the majority of blend modes,
whilst avoiding the known bugs above. Component transfer itself is
still broken, though no moreso than prior to this patch. Fixing
component transfer will be handled in a separate bug.

Differential Revision: https://phabricator.services.mozilla.com/D126967
parent 91f74045
Loading
Loading
Loading
Loading
+7 −5
Original line number Diff line number Diff line
@@ -123,7 +123,7 @@ vec3 LinearToSrgb(vec3 color) {
// This function has to be factored out due to the following issue:
// https://github.com/servo/webrender/wiki/Driver-issues#bug-1532245---switch-statement-inside-control-flow-inside-switch-statement-fails-to-compile-on-some-android-phones
// (and now the words "default: default:" so angle_shader_validation.rs passes)
vec4 ComponentTransfer(vec4 colora, ivec4 vfuncs, int table_address) {
vec4 ComponentTransfer(vec4 colora, int vfuncs, int table_address) {
    // We push a different amount of data to the gpu cache depending on the
    // function type.
    // Identity => 0 blocks
@@ -139,10 +139,12 @@ vec4 ComponentTransfer(vec4 colora, ivec4 vfuncs, int table_address) {
    vec4 texel;
    int k;

    // Dynamically indexing a vector is buggy on some platforms, so use a temporary array
    int[4] funcs = int[4](vfuncs.r, vfuncs.g, vfuncs.b, vfuncs.a);
    for (int i = 0; i < 4; i++) {
        switch (funcs[i]) {
        // Each function value is packed in to 4 bits, with the "r" function at bits 15-12
        // and the "a" function at bits 3-0.
        int func = (vfuncs >> (12 - (i * 4))) & 0xf;

        switch (func) {
            case COMPONENT_TRANSFER_IDENTITY:
                break;
            case COMPONENT_TRANSFER_TABLE:
@@ -186,7 +188,7 @@ void CalculateFilter(
    int table_address,
    vec4 color_offset,
    mat4 color_mat,
    ivec4 v_funcs,
    int v_funcs,
    out vec3 color,
    out float alpha
) {
+8 −14
Original line number Diff line number Diff line
@@ -28,7 +28,12 @@ flat varying ivec2 v_op_table_address_vec;
#define v_table_address v_op_table_address_vec.y

flat varying mat4 v_color_mat;
flat varying ivec4 v_funcs;
// The function to use for each component of a component transfer filter. Using a int[4]
// or vec4 (with each element or component containing the function for each component) has
// ran in to bugs 1695912 and 1731758, so instead we pack each value in to 4 bits of an
// integer. However, due to bug 1630356 we cannot simply use an int, so instead use the
// x component of an ivec2.
flat varying ivec2 v_funcs;
flat varying vec4 v_color_offset;

#ifdef WR_VERTEX_SHADER
@@ -62,20 +67,9 @@ void brush_vs(
    float amount = float(prim_user_data.z) / 65536.0;

    v_op = prim_user_data.y & 0xffff;
    v_funcs.x = (prim_user_data.y >> 16) & 0xffff;
    v_amount = amount;

    // This assignment is only used for component transfer filters but this
    // assignment has to be done here and not in the component transfer case
    // below because it doesn't get executed on Windows because of a suspected
    // miscompile of this shader on Windows. See
    // https://github.com/servo/webrender/wiki/Driver-issues#bug-1505871---assignment-to-varying-flat-arrays-inside-switch-statement-of-vertex-shader-suspected-miscompile-on-windows
    // default: just to satisfy angle_shader_validation.rs which needs one
    // default: for every switch, even in comments.
    v_funcs.r = (prim_user_data.y >> 28) & 0xf; // R
    v_funcs.g = (prim_user_data.y >> 24) & 0xf; // G
    v_funcs.b = (prim_user_data.y >> 20) & 0xf; // B
    v_funcs.a = (prim_user_data.y >> 16) & 0xf; // A

    SetupFilterParams(
        v_op,
        amount,
@@ -105,7 +99,7 @@ Fragment brush_fs() {
        v_table_address,
        v_color_offset,
        v_color_mat,
        v_funcs,
        v_funcs.x,
        color,
        alpha
    );