Skip to content

Apply system code-page encoding to CheckBox and Popup strings on Windows#105

Draft
zzzzzz9125 wants to merge 3 commits into
virtualritz:masterfrom
zzzEffects:master
Draft

Apply system code-page encoding to CheckBox and Popup strings on Windows#105
zzzzzz9125 wants to merge 3 commits into
virtualritz:masterfrom
zzzEffects:master

Conversation

@zzzzzz9125

Copy link
Copy Markdown

Hi,

I'm the maintainer of VideoFX-rs and zzzFX, downstream projects that use this crate to build After Effects / Premiere Pro plugins with localized parameter UI.

The bug

When the plugin runs on Chinese (or Japanese / Korean) Windows, parameter names on the left side of the Effect Controls panel display correctly, but checkbox labels and popup menu items on the right side show garbled text. e.g. 反转颜色 becomes 鍙嶈浆棰滆壊.

Wrong text:
wrong text

Correct text after the fix:
correct text

Root cause

ParamDef::set_name() has always applied system-code-page encoding for the inline name[32] array — on Windows it goes through WideCharToMultiByte(CP_OEMCP, ...), so the host receives bytes it can interpret correctly.

CheckBoxDef::set_label(), PopupDef::set_options(), and the impl String setter generated by define_param_wrapper! do not apply this conversion. They pass raw UTF-8 bytes to the host via CString::new(). The host interprets those bytes as system-code-page (e.g. CP936/GBK on Chinese Windows), so each multi-byte UTF-8 sequence maps to multiple wrong characters.

This is a Windows-only issue. macOS uses UTF-8 natively.

The fix

Added two helpers (encode_to_system_cstring() and encode_to_system_bytes()) that replicate the existing encoding logic from set_name(), then applied them where they're needed.

Caveats

This is still a draft because:

  • Tested on: Windows, Adobe After Effects 25.6.4 only.
  • Expected to work: all versions before AE 26.2. I've heard that AE 26.2 changed the plugin parameter string parsing in a way that may cause additional encoding issues even for plugins that were previously correct. Given that Adobe may potentially fix the new issue brought about by 26.2, my fix targets the pre-26.2 behaviour.
  • Not yet tested on: Premiere Pro, or AE 26.2+.

When I have free time later, I'll conduct tests on other versions and eventually change the status from draft to ready for review.

Copilot AI review requested due to automatic review settings June 5, 2026 07:47
@zzzzzz9125
zzzzzz9125 marked this pull request as draft June 5, 2026 07:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds Windows-specific string encoding so parameter labels and popup options render correctly when using non-ASCII translations (by converting UTF-8 to the system code page).

Changes:

  • Added helpers to encode UTF-8 strings into a system-code-page CString (Windows) while keeping UTF-8 behavior on non-Windows.
  • Updated parameter label setters and popup option building to use the new encoding behavior on Windows.
  • Updated macro-generated setters to use the same encoding path and adjusted short-string length validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.

File Description
after-effects/src/pf/parameters.rs Introduces Windows code-page encoding helpers and applies them to checkbox labels and popup options.
after-effects/src/macros.rs Switches macro-generated string setters to use the encoding helper and updates short-string length checking.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

paste::item! {
pub fn [<set_ $name>](&mut self, v: &str) -> &mut Self {
self.$name = CString::new(v).unwrap();
self.$name = encode_to_system_cstring(v);
pub fn [<set_ $name>](&mut self, v: &str) -> &mut Self {
assert!(v.len() < 32);
let cstr = CString::new(v).unwrap();
let cstr = encode_to_system_cstring(v);
Comment thread after-effects/src/pf/parameters.rs Outdated
use std::ffi::OsStr;
use std::os::windows::ffi::OsStrExt;
unsafe {
use windows_sys::Win32::Globalization::{WideCharToMultiByte, CP_OEMCP};
Comment thread after-effects/src/pf/parameters.rs Outdated
Comment on lines +37 to +38
let len = WideCharToMultiByte(
CP_OEMCP, 0, wstr.as_ptr(), wlen,
Comment thread after-effects/src/pf/parameters.rs Outdated
/// Convert a UTF-8 string to a `CString` encoded in the system code page
/// (e.g. CP936/GBK on Chinese Windows). On macOS this is a no-op since the
/// system uses UTF-8 natively.
fn encode_to_system_cstring(s: &str) -> CString {
Comment thread after-effects/src/pf/parameters.rs Outdated
#[cfg(target_os = "windows")]
{
let bytes = encode_to_system_bytes(s);
CString::new(bytes).unwrap_or_else(|_| CString::new("").unwrap())
Comment thread after-effects/src/pf/parameters.rs Outdated
}
#[cfg(not(target_os = "windows"))]
{
CString::new(s).unwrap_or_else(|_| CString::new("").unwrap())
Comment thread after-effects/src/pf/parameters.rs Outdated
}
encoded.extend_from_slice(&encode_to_system_bytes(opt));
}
self.options = CString::new(encoded).unwrap();
@virtualritz

virtualritz commented Jul 12, 2026

Copy link
Copy Markdown
Owner

@zzzzzz9125 I reviewed and vet-compiled this on both Windows targets (x86_64-pc-windows-gnu and -msvc) -- it builds cleanly and the approach is the right one for #77: using the real Win32 WideCharToMultiByte (matching what ParamDef::set_name already does) rather than a hardcoded GBK table means it generalizes to any system code page (Shift-JIS, etc.), which is strictly better than the GBK-specific snippet in the issue.

Approving in principle. A few non-blocking polish items before this lands:

  1. Doc comment: the CP_OEMCP "equivalent to CP_ACP on CJK Windows" note is misleading in general (OEM vs ANSI differ on Western locales, e.g. 850 vs 1252). Worth softening -- CP_OEMCP is defensible here only because it matches existing set_name behavior.
  2. Dedup: encode_to_system_bytes reimplements the WideCharToMultiByte logic already inline in set_name(); consider refactoring set_name to call the new helper.
  3. Overflow: the ShortString setter still assert!s on the 32-byte limit -- graceful truncation would be friendlier than a panic.
  4. Optionally mirror the macOS CFString conversion in the not(windows) branch for consistency with set_name.

Note: maintainer-edits is off on this PR, so I can't push these tweaks to your branch directly. Could you either enable "Allow edits by maintainers" or apply the tweaks and lift the draft once your Premiere Pro/Ae 26.2 testing is done? Happy to merge as soon as it's marked ready.

@zzzzzz9125

zzzzzz9125 commented Jul 20, 2026

Copy link
Copy Markdown
Author

@virtualritz Sorry for the late reply. I've made a new commit as per your request.

Originally, I planned to complete the testing for 26.2 within these few days, but unfortunately, my PC got waterlogged accidentally and can't work now. So, it's actually still half-repaired.

Regarding the new issue of 26.2, you can see another guy's repo: AE UTF8 Fixer. In short, AE 26.2 has changed the way of reading strings (from ANSI to UTF-8). So, the current fix of mine will actually cause problems with 26.2.

I don't recommend merging it into the main branch at this time, so the status is still a draft. I will continue to complete it once my PC is repaired, maybe a few days later.

Thank you for your patience!

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.

3 participants