vixalien / decibels

Play audio files
https://gitlab.gnome.org/vixalien/decibels
GNU General Public License v3.0
51 stars 17 forks source link

use Snapshot API instead of cairo #64

Open vixalien opened 8 months ago

vixalien commented 8 months ago

fix #61

cc @A6GibKm

A6GibKm commented 8 months ago

What about

diff --git a/src/waveform.ts b/src/waveform.ts
index 892696a..b0da70a 100644
--- a/src/waveform.ts
+++ b/src/waveform.ts
@@ -134,12 +134,6 @@ export class APWaveForm extends Gtk.Widget {

     const leftColor = this.safeLookupColor("accent_color");

-    // Clip the snapshot to the widget area.
-    // Turns out the DrawingArea was automatically doing that for us
-    snapshot.push_clip(
-      new Graphene.Rect({ size: new Graphene.Size({ width, height }) }),
-    );
-
     const indicator = new Graphene.Rect({
       origin: new Graphene.Point({ x: horizCenter, y: 0 }),
       size: new Graphene.Size({ width: LINE_WIDTH, height }),
@@ -169,7 +163,7 @@ export class APWaveForm extends Gtk.Widget {

       // only show 70% of the peaks. there are usually few peaks that are
       // over 70% high, and those get clipped so that not much space is empty
-      const line_height = Math.max(peak * height * 0.7, 1);
+      const line_height = Math.max(peak * height * 0.35, 1);

       const line = new Graphene.Rect({
         origin: new Graphene.Point({
@@ -188,8 +182,6 @@ export class APWaveForm extends Gtk.Widget {

       pointer += GUTTER;
     }
-
-    snapshot.pop();
   }

   set position(pos: number) {

The previous code had line_height = peak * height * 0.7, but the name is a bit misleading. This was not used as the height of the line, but rather the height from its center therefore one should have used height * 0.5 rather than height, assuming peak is in in the 0.0-1.0 range. With peak * height * 0.35 the maximum possible height of the entire line is height * 0.7 which fits in the square.

EDIT: Alternatively, use 0.7 as before, but define the rect as

      const line = new Graphene.Rect({
        origin: new Graphene.Point({
          x: pointer,
          y: vertiCenter - line_height / 2.0,
        }),
        size: new Graphene.Size({
          width: LINE_WIDTH,
          height: line_height,
        }),
      });

this will also ensure that the squares are actually 1x1 in their smallest size.

A6GibKm commented 8 months ago

Another thing that you might consider is using rounded rect for the lines so that their edges are slightly rounded, but please ask designers first.

vixalien commented 8 months ago

Another thing that you might consider is using rounded rect for the lines so that their edges are slightly rounded.

I feel like doing this in a follow up MR would be a better idea. I'll implement your suggestion soon

vixalien commented 8 months ago

Your suggestion makes the line just halved. What I want is the graph to be 0.75 larger (because otherwise the peaks will be smaller) and if the peaks become too large, the should get clipped.

vixalien commented 8 months ago

FWIW, the original Decibels multiplied the waveform by 2. You can see this in the following screenshot comparing the new decibels to the old one.

image

vixalien commented 8 months ago

The recent commits ensure the waveform lines are inside the bounds without using clip rects.

A6GibKm commented 8 months ago

Your suggestion makes the line just halved. What I want is the graph to be 0.75 larger (because otherwise the peaks will be smaller) and if the peaks become too large, the should get clipped.

I would suggest to draw a border of size width x height on the widget as a sanity check. Its important to get everything fitting inside the canvas.

A6GibKm commented 8 months ago

Another thing, is that if it is too small, it is a better idea to give more space to the widget than to potentially go out of the canvas.