Skip to content

Pebble Draw Command preview and export#1587

Open
sockeye-d wants to merge 8 commits into
MewPurPur:mainfrom
sockeye-d:pbw-export
Open

Pebble Draw Command preview and export#1587
sockeye-d wants to merge 8 commits into
MewPurPur:mainfrom
sockeye-d:pbw-export

Conversation

@sockeye-d
Copy link
Copy Markdown
Contributor

@sockeye-d sockeye-d commented Jan 28, 2026

The Pebble Draw Command (PDC) format is a vector format used by the Pebble smartwatches to draw scalable graphics. This PR adds export support for it, as well as extending the preview panel to look similar what the images would like like on real hardware. This includes quantization to 64 colors and snapping vertices to 1 or 1/16th pixels, depending on the precision mode.

The implementation was based on svg2pdc.

More information: https://developer.rebble.io/tutorials/advanced/vector-animations/#drawing-a-pdc-image, https://developer.rebble.io/guides/app-resources/pdc-format/

@MewPurPur
Copy link
Copy Markdown
Owner

The StreamPeer and StreamPeerBuffer classes are currently disabled in .github/disabled_classes.gdbuild as this PR is the first time they are used in the codebase. They need to be enabled for the artifacts to build correctly.

@sockeye-d
Copy link
Copy Markdown
Contributor Author

It should be possible for me to make it use the PackedByteArray buffer directly without StreamPeer, I just prefer the API of StreamPeer. Would that be a better solution?

@MewPurPur
Copy link
Copy Markdown
Owner

Whatever you prefer.

@sockeye-d
Copy link
Copy Markdown
Contributor Author

Should all be fixed now

@MewPurPur
Copy link
Copy Markdown
Owner

MewPurPur commented Feb 9, 2026

Either still hide the size of the file for png, jpeg, and webp, or implement it for these formats too.

I have a feeling the tessellation option doesn't work, I changed it and it didn't seem to affect the image or file size in the UI.

One of my SVGs is previewed like this:

image

Are artifacts like these intentional?

Are the two new SVGs needed? I don't know what those could be used for, and they seem to be unused at the moment.

@MewPurPur
Copy link
Copy Markdown
Owner

MewPurPur commented Feb 9, 2026

I'm against adding PDC-specific things in previews in this PR. We can do it later, but I think we should first investigate the idea of having utilities to preview every format, and to configure the previews just like in the image export menu.

@sockeye-d
Copy link
Copy Markdown
Contributor Author

Right, should all be fixed now

@MewPurPur
Copy link
Copy Markdown
Owner

MewPurPur commented Feb 19, 2026

Could you rebase? I made some changes to the CI that make it so changing the disabled_classes file should result in new cache.

Edit: Nevermind, the CI needs more work

@MewPurPur
Copy link
Copy Markdown
Owner

MewPurPur commented Feb 21, 2026

The issues have been fixed, seems like they might have been from Godot 4.6.1. Could you rebase now?

@sockeye-d
Copy link
Copy Markdown
Contributor Author

Rebased!

@MewPurPur
Copy link
Copy Markdown
Owner

It seems like transforms aren't accounted for, is this intentional?

image image

@MewPurPur
Copy link
Copy Markdown
Owner

MewPurPur commented Feb 22, 2026

Other notes:

  • The size label is pretty nice, let's keep it for all formats! But tune down the export menu's VBoxes' spacings slightly, as webp is now so tall it shifts the layout
  • Add the new fields to the focus sequence in _ready()
  • The correct spelling is "tessellation" / "tessellate"
  • Align the precision dropdown right next to the "Precise:" label

@MewPurPur
Copy link
Copy Markdown
Owner

I actually realized the reason why the other formats have no size indication - there's no way to get one without converting the image, and there's no way to convert the image without a lag spike. Previews are limited to 512x512 for this reason. So it would either need to be reverted to how it was, or have some kind of indication that it's not the real deal for bigger images - I can't think of a good one.

The two SVGs in the PR seem cool, but are now unused if I'm not mistaken? If they are still necessary, import them as DPITexture.

@sockeye-d
Copy link
Copy Markdown
Contributor Author

I could estimate the size of the image with the actual resolution by extrapolating the
clamped image's bytes per pixel to the full image's number of pixels, probably. Then I could add an " (estimated)" suffix when that happens.

The two SVGs are unused, but they were for the PDC preview checkbox in the previews panel — if we plan on bringing that back, I'll still need them.

@MewPurPur
Copy link
Copy Markdown
Owner

Hmm, okay, I have a different idea, but let's still keep them until then, just reimport both as DPITexture.

As for size estimation, I think that's a good idea!

@sockeye-d
Copy link
Copy Markdown
Contributor Author

All fixed now. Would there be any interest in generating the export on a separate thread, then showing a throbber or something while it loads? That would sort of fix the slowness issue

@MewPurPur
Copy link
Copy Markdown
Owner

I tried that, but I needed a separate threadless solution for web and the complexity didn't feel worth it.

Will test in a bit in my lunch break.

@MewPurPur
Copy link
Copy Markdown
Owner

One more rebase, please? I broke context popups right as you were rebasing :(

@sockeye-d
Copy link
Copy Markdown
Contributor Author

Rebased on top of upstream/main

@MewPurPur
Copy link
Copy Markdown
Owner

MewPurPur commented Mar 3, 2026

Rectangles are broken and the "Precise:" section should be left-aligned. I'll go over the code in a bit.

Copy link
Copy Markdown
Owner

@MewPurPur MewPurPur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other notes: The ThemeUtils changes should be reverted. They aren't utilized yet and they seem very hacky.

Edit: Also, maybe pull the whole export size estimation inside an if else block after checking if the format is SVG? Right now you have a bug where big SVGs will trigger the "approximate" label, and also have this weird match setup.

Comment on lines +104 to +118
var focus_sequence: Array[Control]
focus_sequence.append(clipboard_button)
focus_sequence.append(format_dropdown)
focus_sequence.append(path_quality_edit)
focus_sequence.append(precise_path_mode_dropdown)
focus_sequence.append(lossless_checkbox)
focus_sequence.append(quality_edit)
focus_sequence.append(scale_edit)
focus_sequence.append(width_edit)
focus_sequence.append(height_edit)
focus_sequence.append(cancel_button)
focus_sequence.append(export_button)

HandlerGUI.register_focus_sequence(self, focus_sequence, true)
clipboard_button.grab_focus(true)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer that this doesn't get changed.

func update() -> void:
# Determine which fields are visible.
quality_related_container.visible = export_data.format in ["jpg", "jpeg", "webp"]
path_quality_container.visible = export_data.format in ["pdc"]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
path_quality_container.visible = export_data.format in ["pdc"]
path_quality_container.visible = (export_data.format == "pdc")

export_size = roundi(texture_preview.last_image_size * maxf(1.0, export_size_fac))
final_size_label.text = Translator.translate("Size") + ": " + String.humanize_size(export_size)
if export_size_fac > 1.0:
final_size_label.text += Translator.translate(" (approximate)")
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on current conventions, only the "approximate" string should be translated and not the surrounding punctuation

Comment thread project.godot

window/size/viewport_width=1040
window/size/viewport_height=650
window/size/mode=2
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this about?

Comment thread src/utils/SVGPathUtils.gd
Comment on lines +160 to +165
static func generate_ellipse(
start: Vector2, end: Vector2, r: Vector2,
rot: float,
large_arc_flag: bool, sweep_flag: bool,
path_generator: PathGenerator = default_path_generator
) -> PackedVector2Array:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the params in a single line like the rest of the codebase, also rename to generate_elliptical_arc.

Comment on lines +101 to +104
size = Vector2i(
sp.get_16(),
sp.get_16(),
)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do on one line.

## [br][br]
## Implementation based on the specification at [url]https://developer.rebble.io/guides/app-resources/pdc-format/[/url] and
## [url=https://github.com/pebble-examples/cards-example/blob/master/tools/svg2pdc.py]svg2pdc.py[/url].
class_name PDCImage
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it meant to extend RefCounted (as by default)? If so, I think this should be explicit.

Comment on lines +273 to +275
var x := sp.get_16()
var y := sp.get_16()
center = Vector2(x, y)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say no need for intermediate variables.

Comment on lines +287 to +289
var x := float(sp.get_16()) / (1 << 3)
var y := float(sp.get_16()) / (1 << 3)
points.append(Vector2(x, y))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say no need for intermediate variables. Divide the whole thing by (1 << 3).

Comment on lines +253 to +255
var x := sp.get_16()
var y := sp.get_16()
points.append(Vector2(x, y))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say no need for intermediate variables.

@MewPurPur
Copy link
Copy Markdown
Owner

I also found that curves no longer have tangents to their control points.

@MewPurPur
Copy link
Copy Markdown
Owner

The export menu was reworked, which requires significant changes for the PR. But it should overall fit into the new systems way better than it did with the old ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants