After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 759080 - Tiled rendering for ClutterCanvas
Tiled rendering for ClutterCanvas
Status: RESOLVED OBSOLETE
Product: clutter
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2015-12-06 02:33 UTC by Lionel Landwerlin
Modified: 2021-06-10 11:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
canvas: only destroy CoglBuffer if size is changed (3.12 KB, patch)
2015-12-06 02:35 UTC, Lionel Landwerlin
accepted-commit_now Details | Review
canvas: implement a tiled based rendering (27.68 KB, patch)
2015-12-06 02:35 UTC, Lionel Landwerlin
none Details | Review
canvas: implement a tiled based rendering (27.71 KB, patch)
2015-12-06 02:38 UTC, Lionel Landwerlin
none Details | Review
canvas: implement a tiled based rendering (27.71 KB, patch)
2015-12-06 02:41 UTC, Lionel Landwerlin
needs-work Details | Review
test: interactive: port easing test to ClutterCanvas (2.61 KB, patch)
2015-12-07 00:38 UTC, Lionel Landwerlin
rejected Details | Review
Screenshot of the tiles in Totem (111.92 KB, image/png)
2015-12-10 11:49 UTC, Lionel Landwerlin
  Details

Description Lionel Landwerlin 2015-12-06 02:33:42 UTC
As we move torwards hidpi displays, our widgets gets bigger in size. In
particular when embedded in ClutterActor, we end up uploading a lot of
pixels which may not necessarly have changed.

It would be great if we could just reupload the areas that have been
actually redrawn. Unfortunetaly most GPUs are pretty crap at doing that, so
we most people have settled for putting CPU generated graphics into tiles
and reupload only tiles that changes.

Here is an attempt at using tiles for ClutterCanvas. This is still untested
from a performance point of view. Hopefully I'll some numbers at some point
and a nice test to add.
Comment 1 Lionel Landwerlin 2015-12-06 02:35:08 UTC
Created attachment 316827 [details] [review]
canvas: only destroy CoglBuffer if size is changed
Comment 2 Lionel Landwerlin 2015-12-06 02:35:14 UTC
Created attachment 316828 [details] [review]
canvas: implement a tiled based rendering
Comment 3 Lionel Landwerlin 2015-12-06 02:38:27 UTC
Created attachment 316829 [details] [review]
canvas: implement a tiled based rendering
Comment 4 Lionel Landwerlin 2015-12-06 02:41:25 UTC
Created attachment 316830 [details] [review]
canvas: implement a tiled based rendering
Comment 5 Lionel Landwerlin 2015-12-07 00:38:55 UTC
Created attachment 316859 [details] [review]
test: interactive: port easing test to ClutterCanvas
Comment 6 Lionel Landwerlin 2015-12-07 00:40:09 UTC
Review of attachment 316859 [details] [review]:

Wrong bug...
Comment 7 Lionel Landwerlin 2015-12-10 11:49:27 UTC
Created attachment 317101 [details]
Screenshot of the tiles in Totem

Just showing up how the tiles are allocated.
Comment 8 Emmanuele Bassi (:ebassi) 2015-12-10 12:17:03 UTC
Review of attachment 316827 [details] [review]:

Looks good.

::: clutter/clutter-canvas.c
@@ +505,2 @@
     {
       cogl_object_unref (priv->buffer);

Should probably replace the unref() + NULL-ify with g_clear_pointer().
Comment 9 Lionel Landwerlin 2015-12-10 12:21:14 UTC
Thanks, will do.

I noticed a problem yesterday with the repeat mode, it's drawing a fraction of a pixel outside the content box. That messes with the clipped redraws. I'll fix this asap.
Comment 10 Emmanuele Bassi (:ebassi) 2015-12-10 12:26:54 UTC
Review of attachment 316830 [details] [review]:

::: clutter/clutter-canvas.c
@@ +125,3 @@
 
+static CanvasTile *
+canvas_tile_new (int x1, int y1, int x2, int y2, int scale)

Coding style: please split arguments into separate lines.

@@ +135,3 @@
+
+  tile->region.x = x1 / scale;
+  tile->real_region.x = x1;

I'd convert the values to doubles and then floor() and ceil() the values, otherwise we'll likely get off-by-one rounding errors.

@@ +197,3 @@
+  ClutterPaintNode *node = clutter_texture_node_new (tile->texture, color,
+                                                     min_filter, mag_filter);
+                                            tile->real_region.height,

Should probably add the tile id to the name.

@@ +767,3 @@
+      CanvasTile *tile = g_ptr_array_index (priv->tiles, i);
+
+  ClutterCanvasPrivate *priv = self->priv;

Emitting the ::draw signal multiple times may be problematic, especially if the caller code has no way to distinguish a tiled draw from a non-tiled one.

We should add a function that allows users to query the tile rectangle being drawn with regards to the content area, so that they can decide to discard content from their own side; Cairo is not always able to optimize discarded content.

Alternatively, if the tile size is non-zero then we may need a different signal, like ::draw-tile, which provides the geometry of the tile instead of just the width/height of the surface.
Comment 11 André Klapper 2021-06-10 11:31:55 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version of clutter, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a ticket at
  https://gitlab.gnome.org/GNOME/clutter/-/issues/

Thank you for your understanding and your help.