Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve fractional values for colors #1432

Open
u1f992 opened this issue Dec 18, 2024 · 12 comments
Open

Preserve fractional values for colors #1432

u1f992 opened this issue Dec 18, 2024 · 12 comments

Comments

@u1f992
Copy link

u1f992 commented Dec 18, 2024

Is your feature request related to a problem? Please describe.
In the Chromium used by the latest version of the Vivliostyle CLI, fractional numbers for colors, such as rgb(0.0 0.1 0.2), appear to also influence the color rendering in the PDF output.

$ gs -dPDFDEBUG -dBATCH -dNOPAUSE -sDEVICE=nullpage chromium.pdf 2>&1 | grep rg
 0 0.000400 0.000800 rg

However, when using Vivliostyle, these fractional values seem to be rounded during processing, resulting in reduced color precision.

$ vivliostyle build index.html -o vivliostyle.pdf
$ gs -dPDFDEBUG -dBATCH -dNOPAUSE -sDEVICE=nullpage vivliostyle.pdf 2>&1 | grep "rg"
 0 0 0 rg

Describe the solution you'd like
It would be preferable for Vivliostyle to preserve fractional color values as-is, without rounding, during processing.
This would ensure higher color fidelity in the PDF output, aligning Vivliostyle's behavior with that of Chromium and improving the overall quality of the generated PDFs.

@u1f992
Copy link
Author

u1f992 commented Dec 18, 2024

I identified the root cause of the issue:

elemStyle.setProperty(prefixed, value);

> document.write('<div style="display: inline-block; width: 100px; height: 100px">')
> const elem = document.getElementsByTagName("div")[0]
> elem.style.setProperty("background-color", "rgb(0 0.1 0.2)")
> elem.outerHTML
'<div style="display: inline-block; width: 100px; height: 100px; background-color: rgb(0, 0, 0);"></div>'

Using elem.style["background-color"] = "rgb(0 0.1 0.2)", elem.style.backgroundColor = "rgb(0 0.1 0.2)", or "rgb(0, 0.1, 0.2)" (comma-separated) all yield the same result and do not resolve the issue.

At this point, it is unclear whether this behavior is a Chromium bug or the intended behavior as per the specification.

For reference:

The only workaround I could find is to use getAttribute and setAttribute:

> elem.setAttribute("style", elem.getAttribute("style") + `; background-color: rgb(0 0.1 0.2);`)
'<div style="display: inline-block; width: 100px; height: 100px; background-color: rgb(0 0.1 0.2);"></div>'

I initially thought that Vivliostyle rounding off fractionals is an intended behavior, but tracing the root cause suggests otherwise. This issue now should be labeled as "bug."

@MurakamiShinyu
Copy link
Member

Related CSS spec: https://drafts.csswg.org/css-color-4/#css-serialization-of-srgb

The precision with which sRGB component values are retained, and thus the number of significant figures in the serialized value, is not defined in this specification, but must at least be sufficient to round-trip eight bit values. Values must be rounded towards +∞, not truncated.

So the Chromium's behavior in which the values are rounded to eight bit on serialization is not great, but not wrong.

Fractional values in rgb() were introduced in CSS Color Level 4. That was implemented in Chromium, Implement CSS Color Level 4, but with the following limitation yet:

This CL is not addressing other potential issues where the RGBA values
are being casted to int, and therefore losing potential precision when
specified as percentages. That will be handled in a different CL.

@u1f992
Copy link
Author

u1f992 commented Dec 19, 2024

Thank you for the clarification! This is exactly the document I was looking for.

Then, if we make the following changes for example, is there anything we should consider?

diff --git a/packages/core/src/vivliostyle/base.ts b/packages/core/src/vivliostyle/base.ts
index 43722e911..afe1b35d5 100644
--- a/packages/core/src/vivliostyle/base.ts
+++ b/packages/core/src/vivliostyle/base.ts
@@ -384,17 +384,21 @@ export function getPrefixedPropertyNames(prop: string): string[] | null {
   return null;
 }
 
+function appendStyleString(elem: Element, prop: string, value: string) {
+  const existingStyle = (elem.getAttribute("style") || "").replace(/;\s*$/, "");
+  elem.setAttribute("style", `${existingStyle}; ${prop}: ${value};`);
+}
+
 export function setCSSProperty(
   elem: Element,
   prop: string,
   value: string,
 ): void {
-  const elemStyle = (elem as HTMLElement)?.style;
-  if (!elemStyle) {
+  if (!elem) {
     return;
   }
   if (prop.startsWith("--")) {
-    elemStyle.setProperty(prop, value || " ");
+    appendStyleString(elem, prop, value || " ");
     return;
   }
   const prefixedPropertyNames = getPrefixedPropertyNames(prop);
@@ -414,12 +418,12 @@ export function setCSSProperty(
         switch (value) {
           case "all":
             // workaround for Chrome 93 bug https://crbug.com/1242755
-            elemStyle.setProperty("text-indent", "0");
+            appendStyleString(elem, "text-indent", "0");
             break;
         }
         break;
     }
-    elemStyle.setProperty(prefixed, value);
+    appendStyleString(elem, prefixed, value);
   }
 }

@MurakamiShinyu
Copy link
Member

I see. It may work. Could you make a PR?

@u1f992
Copy link
Author

u1f992 commented Dec 19, 2024

調査メモ:setPropertyを実行するたびに丸めが発生します。

> const elem = document.createElement("div")
> elem.setAttribute("style", "color: rgb(0 0.1 0.2)")
> elem.outerHTML
'<div style="color: rgb(0 0.1 0.2)"></div>'

> elem.style.setProperty("display", "inline-block")
> elem.outerHTML
'<div style="color: rgb(0, 0, 0); display: inline-block;"></div>'

@u1f992
Copy link
Author

u1f992 commented Dec 23, 2024

調査メモ:CSS変数--neverが宣言されていないと仮定し、rgb(var(--never, 0.1) 2 3)として意図的にフォールバック値を使用させることで、インラインスタイルでも実数値を使用できるようです。

> document.body.style.setProperty("background-color", "rgb(var(--never, 0.1) 2 3)")
> document.body.outerHTML
<body style="background-color: rgb(var(--never, 0.1) 2 3);"></body>
$ gs -dPDFDEBUG -dBATCH -dNOPAUSE -sDEVICE=nullpage never.pdf 2>&1 | grep rg
 0.000400 0.007800 0.011800 rg

@MurakamiShinyu
Copy link
Member

MurakamiShinyu commented Dec 23, 2024

color() 関数を使う手もあります。例: color(srgb 0.0004 0.0078 0.0118)

See https://drafts.csswg.org/css-color-4/#color-function

@u1f992
Copy link
Author

u1f992 commented Dec 23, 2024

なるほど! color()の場合は255で割ることになりますが、PDF上では依然十分な精度を保つことができそうです。

> document.body.style.setProperty("background-color", `color(srgb ${0.1/255} ${2/255} ${3/255})`)
> document.body.outerHTML
'<body style="background-color: color(srgb 0.000392157 0.00784314 0.0117647);"></body>'
$ gs -dPDFDEBUG -dBATCH -dNOPAUSE -sDEVICE=nullpage color-srgb.pdf 2>&1 | grep rg
 0.000400 0.007800 0.011800 rg

@MurakamiShinyu
Copy link
Member

PR #1434 のコメント:
#1434 (comment)

Chromiumの修正を求めるよりも、Vivliostyleで回避策を実装する方がはるかに現実的であると考えています。
インラインスタイルにおいて実数を含むrgb()が8ビット整数のレガシー記法に正規化される件については、ChromiumだけでなくFirefoxでも同様の挙動になっています。これは互換性を重視した暗黙の合意による挙動であると推測しており、Webブラウザが主に画面表示を目的としていることを考えれば、合理的な妥協だとも理解しています。
既存のJavaScriptコードの互換性を考慮すると、この件が問題として認識されるかどうかについて確証がありません。

CSSWGやChromiumの開発者たちは明らかにこの問題を認識していると思います。CSS Color Level 4 §15.2.2. CSS serialization of sRGB values に、シリアライズの結果が rgba(178.5, 93.5, 51, 0.5) のような非整数のRGB値になる例が示されています。そして、JavaScriptコードの開発者向けの注意として

NOTE: authors of scripts which expect color values returned from getComputedStyle to have <integer> component values, are advised to update them to also cope with <number>.

と書かれています。

Chromiumの開発者がRGB値の精度が失われる問題を認識しているかですが、前のコメントで引用したChromium issueの Implement CSS Color Level 4 にある Chromium のコミットの次の記述や、

This CL is not addressing other potential issues where the RGBA values
are being casted to int, and therefore losing potential precision when
specified as percentages. That will be handled in a different CL.

ChromiumソースコードにTODOとして書かれている次の記述から、

https://github.com/chromium/chromium/blob/main/third_party/blink/renderer/platform/graphics/color.h#L178

// TODO(crbug.com/ 1333988): We have to reevaluate how we input int RGB and
// RGBA values into blink::color. We should remove the int inputs in the
// interface, to avoid callers to have the double values and convert them to
// int for then being converted again internally to float. We should deprecate
// FromRGB, FromRGBA and FromRGBAFloat methods, to only allow for
// FromRGBALegacy. We could also merge all the constructor methods to one
// CreateColor(colorSpace, components...) method, that will internally create
// methods depending of the color space and properly store the none-ness of
// the components.

RGB値の精度が失われる問題を将来的に解決したいのではないかと推測します。
ただChromiumのissue https://issues.chromium.org/ を検索して直接 rgb() のシリアライズ精度についての issue は見つからないので、新たに issue として登録する意味はあるだろうと思います。

Chromiumでこれが解決されるまでは、精度が重要な色の指定をする場合には color(srgb …) を使うようにするという運用が現実的かと思います。

@u1f992
Copy link
Author

u1f992 commented Dec 23, 2024

ありがとうございます。Chromiumのソースコードにも言及があるのですね。
ChromiumにIssueを登録しました。進展がありましたらお知らせします。
https://issues.chromium.org/issues/385613195

Chromiumでこれが解決されるまでは、精度が重要な色の指定をする場合にはcolor(srgb …)を使うようにするという運用が現実的かと思います。

私の目的はこの記法で達成されたため、本件に関してこれ以上PRを提出する予定はありません。

@MurakamiShinyu
Copy link
Member

ChromiumにIssueを登録しました。進展がありましたらお知らせします。
https://issues.chromium.org/issues/385613195

Issueのタイトル
Fractional values in rgb() for inline styles are serialized to legacy syntax unexpectedly
に関してですが、

問題は legacy syntax になることではなくて、値が整数に丸められて精度が失われることだということを明記するとよいかと思います。

コンマ区切りのレガシー形式にシリアライズされることは CSS Color Level 4 §15.2.2. CSS serialization of sRGB values の仕様どおりです。

求めることは、コンマ区切りの rgb() であっても、小数点のついた値が出力されるということです。

つまり、 rgb(0 0.1 0.2) に対するシリアライズの結果が現在は rgb(0, 0, 0) となるのですが、expected behaviorは rgb(0, 0.1, 0.2) であるということを書くとよいと思います。

@u1f992
Copy link
Author

u1f992 commented Dec 23, 2024

助言までいただきありがとうございます!Chromium Issue Trackerに慣れておらず操作がよくわからないのですが、編集はできないようなのでまずはコメントで補足しました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants