D29131: [scene] Generate window quads for sub-surfaces

apol added a comment.


  +1
  Looks good to me overall (not that I'm overly familiar with subsurfaces), commented some nitpicks.

INLINE COMMENTS

> scene_opengl.cpp:1271
> +
> +    if (!renderNodes[context.shadowOffset].quads.isEmpty()) {
> +        SceneOpenGLShadow *shadow = static_cast<SceneOpenGLShadow *>(m_shadow);

For readability it would make sense to keep a reference of the objects we're interacting with.

  auto &shadowRenderNode = renderNodes[context.shadowOffset];
  if (!shadowRenderNode.quads.isEmpty())
  ...

> scene_opengl.cpp:1288
>  
> -    nodes[ContentLeaf].texture = s_frameTexture;
> -    nodes[ContentLeaf].hasAlpha = !isOpaque();
> -    // TODO: ARGB crsoofading is atm. a hack, playing on opacities for two dumb SrcOver operations
> -    // Should be a shader
> +    // FIXME: Cross-fading must be implemented in a shader.
> +    float contentOpacity = data.opacity();

Doesn't look like a fixme, more of a phabricator task to tackle in the future?

Reading this makes others feel sad but doesn't make one implement anything.

> scene.cpp:1035
> +
> +        for (const QRectF &rect : region) {
> +            // Note that the window quad id is not unique if the window is shaped, i.e. the

How about

  for (const QRectF &rect : region) {
    const QRect positioned = rect.translated(position);
    const QRect scaled = rect * scale;
  
    quad[0] = WindowVertex(positioned.topLeft(), scaled.topLeft());
    quad[1] = WindowVertex(positioned.topRight(), scaled.topRight());
    quad[2] = WindowVertex(positioned.bottomRight(), scaled.bottomRight());
    quad[3] = WindowVertex(positioned.bottomLeft(), scaled.bottomLeft());
  
    quads << quad;
  }

> subsurfacemonitor.cpp:45
> +            this, &SubSurfaceMonitor::subSurfaceResized);
> +#if 0 // Add SurfaceInterface::mapped()
> +    connect(surface, &SurfaceInterface::mapped,

Why's that commented?

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D29131

To: zzag, #kwin
Cc: apol, ngraham, kwin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, abetts, sebas, ahiemstra, mart