use Snapshot API instead of cairo #64
Conversation
vixalien
force-pushed
the
wip/vixalien/snapshots-direct
branch
from
be9200c
to
fc366f9
Compare
src/waveform.ts
Outdated
| ctx.moveTo(horizCenter, vertiCenter - height); | ||
| ctx.lineTo(horizCenter, vertiCenter + height); | ||
| ctx.stroke(); | ||
| // Clip the snapshot to the widget area. |
There was a problem hiding this comment.
In general the more sensible approach is to draw inside this rectangle, this code implies that things will be drawn outside and can appear half-way offscreen.
There was a problem hiding this comment.
You're correct. The reason is that the peaks are drawn "scaled up" by another 70%
Usually the waveform peaks are short (50-70%) of the widget height. I make them a bit larger so that there's not much empty space.
Should I stop clipping the peaks or should I instead clip the waveform lines before they are given their size is larger than widget.height?
There was a problem hiding this comment.
I am not familiar with the code, ideally one does the math and draws them at a size based on height in such a way that even after adding the position it does not go beyond the given size.
Without thinking too much, I would set the rectangle with height factor * height (factor goes from 0 to 1) and a y position given by (height - factor * height) / 2.0 so that it looks centered.
Note that you have not ensure everything passed to the snapshot API (e.g. values for a rectangle) is an integer if you want to avoid blurryness. (just use a floor() fn whenever you do a division or multiple by a random float like factor).
I will try to test or review this with more time in the future.
|
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 EDIT: Alternatively, use 0.7 as before, but define the rect as this will also ensure that the squares are actually 1x1 in their smallest size. |
|
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. |
I feel like doing this in a follow up MR would be a better idea. I'll implement your suggestion soon |
|
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. |
|
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.
|
|
The recent commits ensure the waveform lines are inside the bounds without using clip rects. |
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. |
|
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. |
fix #61
cc @A6GibKm