googlefonts / colr-gradients-spec

63 stars 8 forks source link

Optimize PAINT_COMPOSITE painting algorithm #369

Open behdad opened 1 year ago

behdad commented 1 year ago

Currently the spec says this about the PAINT_COMPOSITE painting algorithm:

 if format 32: // PaintComposite

        // render backdrop sub-graph
        call renderPaint() passing the backdrop child paint table and save
        the result

        // render source sub-graph
        call renderPaint() passing the source child paint table and save
        the result

        // compose source and backdrop
        compose the source and backdrop using the specified composite mode

        // compose final result
        compose the result of the above composition onto the surface using
        simple alpha blending

I believe this can be simplified to need only one group-push and blending instead of two. Observe that at the beginning of every paint call (not just paint-composite, every paint call), the current surface is always clear. As such, we can simply draw the backdrop to the current surface, and only push-group for the source paint, and pop it with the composite mode.

The PAINT_COMPOSITE should really do one compositing more than other paints, not two.

cc @raphlinus since he noticed this inefficiency before I believe.

behdad commented 1 year ago

cc @drott

behdad commented 1 year ago

cc @raphlinus since he noticed this inefficiency before I believe.

https://github.com/googlefonts/colr-gradients-spec/issues/360

drott commented 1 year ago

the current surface is always clear.

Do you mean by that that there are no pixels drawn at the start of every paint call, or only at the root of the tree? Or do you mean that the current surface always has blend mode clear?

Skia implementation is here: Drawn pixel state is kept in the Skia implementation in an SkCanvas that is passed through while traversing the paint graph that already has drawn pixels when entering a PaintComposite operation.

Can the source paint modify alpha values of the backdrop when backdrop and source are composited independently of what's on the canvas state? If yes, in that case drawing the backdrop first, "overpainting" an existing pixel on the canvas with a non-transparent color would erase information from previous layers, which could not be fixed later when compositing the source paint on top.

behdad commented 1 year ago

the current surface is always clear.

Do you mean by that that there are no pixels drawn at the start of every paint call, or only at the root of the tree? Or do you mean that the current surface always has blend mode clear?

There are no pixels drawn on the current canvas at the beginning of every paint call. This can be proved by induction. Note that only leaf paints draw anything; and the only nodes that have more than one paint child are PaintComposite and PaintColrLayers, both of each start a new group for each child. So, at the start of each node in the graph the current canvas is empty.

drott commented 1 year ago

My current thinking is, perhaps this could be simplified if we would want to introduce a backwards compat break, and introduce a PaintCompositeWithOnlyOneLayer that saves a layer creation and always merges with what's already below - i.e. letting the current state of the canvas be the backdrop, and the newly to be composited layer be the source - but it's not equivalent to drop the creation of the extra layer for the current PaintComposite. Perhaps with such a new Paint primitive the same expressiveness could be achieved (with certain caveats or extra clips if required).

There are no pixels drawn on the current canvas at the beginning of every paint call. This can be proved by induction. Note that only leaf paints draw anything; and the only nodes that have more than one paint child are PaintComposite and PaintColrLayers, both of each start a new group for each child. So, at the start of each node in the graph the current canvas is empty.

It's true what you're saying. But: If we drop creating the first layer, then create a layer for the src paint, draw it, including transforms and clipping operations, then when compositing that layer down - the whole existing canvas under it would be taken into account. The canvas would not be empty. This leads to a different results compared to compositing src down to only backdrop, then alpha blending the result of those.

Example

Consider the srcIn example in the test font, gid 123:

Source In means: "The source that overlaps the destination, replaces the destination." My addition: "Clipped out pixels / Everything else is dropped".

Glyph TTX: It's PaintColorLayers at the top level, consisting of the following two layers:

      <Paint index="10" Format="10"><!-- PaintGlyph -->
        <Paint Format="2"><!-- PaintSolid -->
          <PaletteIndex value="10"/>
          <Alpha value="1.0"/>
        </Paint>
        <Glyph value="cross_glyph"/>
      </Paint>
      <Paint index="11" Format="32"><!-- PaintComposite -->
        <SourcePaint Format="22"><!-- PaintScaleUniformAroundCenter -->
          <Paint Format="10"><!-- PaintGlyph -->
            <Paint Format="2"><!-- PaintSolid -->
              <PaletteIndex value="11"/> <!-- lightblue -->
              <Alpha value="1.0"/>
            </Paint>
            <Glyph value="upem_box_glyph"/>
          </Paint>
          <scale value="0.5"/>
          <centerX value="667"/>
          <centerY value="333"/>
        </SourcePaint>
        <CompositeMode value="src_in"/>
        <BackdropPaint Format="22"><!-- PaintScaleUniformAroundCenter -->
          <Paint Format="10"><!-- PaintGlyph -->
            <Paint Format="2"><!-- PaintSolid -->
              <PaletteIndex value="12"/> <!-- yellow -->
              <Alpha value="1.0"/>
            </Paint>
            <Glyph value="upem_box_glyph"/>
          </Paint>
          <scale value="0.5"/>
          <centerX value="333"/>
          <centerY value="667"/>
        </BackdropPaint>
      </Paint>

Result without an extra offscreen layer for backdrop:

image

Drawing steps / description:

The yellow rect is painted on top of the cross. A new layer is created, which contains the blue rect. That is composited down with srcIn, the top and left part of the cross are dropped, as there is no overlap with blue rect, the central rect overlap, and the the right and bottom parts of the cross, that overlapped with the blue rect, are filled with blue.

Result with extra offscreen layer for backdrop and offscreen compositing of backdrop and src:

image

Drawing steps / description:

The yellow rect, and the blue rect overlap in a center rect area. Pixels are filled with blue, that is composited down.

So the result is not identical, and I don't think at this point we can redefine PaintComposite to mean something else without breakage.

behdad commented 1 year ago

In HarfBuzz I'm getting the correct rendering without the extra push/pop. So, we're doing something differently it seems.

behdad commented 1 year ago

PaintColrLayers, both of each start a new group for each child.

I think your PaintColrLayers doesn't start a new group for each child then? I think that's the difference here.

behdad commented 1 year ago

PaintColrLayers, both of each start a new group for each child.

I think your PaintColrLayers doesn't start a new group for each child then? I think that's the difference here.

If you don't, I think you might end up in other issues as well. I'll try to think about it.

behdad commented 1 year ago

PaintColrLayers, both of each start a new group for each child.

I think your PaintColrLayers doesn't start a new group for each child then? I think that's the difference here.

If you don't, I think you might end up in other issues as well. I'll try to think about it.

I think you we only need one or another. NOT starting a new layer for each layer of PaingColrLayers sounds more optimized indeed. Lemme try that.

behdad commented 1 year ago

I confirmed that removing push_group in PaintColrLayer introduces the bug for me. Adding a push_group in PaintComposite then fixes it again.

The documentation for PaintColrLayer says:

Format 1 is used to define a vector of layers. The layers are a slice of layers from the LayerList table. The first layer is the bottom of the z-order, and subsequent layers are composited on top using the COMPOSITE_SRC_OVER composition mode.

which suggests that each layer should be drawn in its own group then composited using SRC_OVER. But I think with SRC_OVER the results will always be the same, hence it's a better optimization IMO.

behdad commented 1 year ago

But the spec needs clarification to say other layers are drawn directly on top?

behdad commented 1 year ago

cc @matthiasclasen

drott commented 1 year ago

which suggests that each layer should be drawn in its own group then composited using SRC_OVER. But I think with SRC_OVER the results will always be the same, hence it's a better optimization IMO.

Yes, my understanding is that SRC_OVER is equivalent to "draw on top", and each "branch out" for a color layer should not cross-talk to the next layer, as each pushed transform and clip needs to be popped before you return to the iteration over the layers and draw the next one.

In fact, I think if the first group is only created in PaintColorLayers, and no extra two groups but only one is opened in PaintComposite, then I think there would be bugs with nested PaintComposites. That would require a new test glyph. Let me know if you need one. Can be added around here: https://github.com/googlefonts/color-fonts/blob/main/config/test_glyphs-glyf_colr_1.py#L1429

behdad commented 1 year ago

In fact, I think if the first group is only created in PaintColorLayers, and no extra two groups but only one is opened in PaintComposite, then I think there would be bugs with nested PaintComposites. That would require a new test glyph. Let me know if you need one. Can be added around here: https://github.com/googlefonts/color-fonts/blob/main/config/test_glyphs-glyf_colr_1.py#L1429

I don't think so, because of the analysis I did in the first comment. But sure would be good to add a test.

behdad commented 1 year ago

I created https://github.com/harfbuzz/harfbuzz/pull/4498 for HarfBuzz to follow your scheme.

behdad commented 1 year ago

Currently the paint algorithm in the spec says:

    if format 1: // PaintColrLayers
        for each referenced child paint table, in bottom-up z-order:
            // for ordering, see Layering; Format 1: PaintColrLayers
            call renderPaint() passing the child paint table

            compose the returned graphic onto the surface using simple
            alpha blending

The last sentence needs to be removed to match what we are discussing here.

drott commented 1 year ago

The last sentence needs to be removed to match what we are discussing here.

I agree the sentence is a bit misleading. It would be clearer and cause less confusion or suggestion to create a layer - but it also says:

Applications are not required to implement rendering using this algorithm, but must produce equivalent results.

And that sentence can IMO also be read to mean: draw it on top with alpha blending. So at least I don't think it's strictly a bug in the spec.