Closed Bug 996108 Opened 8 years ago Closed 7 years ago

Skia does not seem to have translation invariant rasterization

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

Details

(Whiteboard: [skia-upstream])

Attachments

(4 files)

This shows up in a bunch of reftests. I have no idea why yet.
What does the spec say about float-to-int casting?
Flags: needinfo?(botond)
(In reply to Benoit Jacob [:bjacob] from comment #2)
> What does the spec say about float-to-int casting?

Section 4.9, "Floating-integral conversions", paragraph 1: "... The conversion truncates; that is, the fractional part is discarded. The behavior is undefined if the truncated value cannot be represented in the destination type,"

Is this what you were looking for?
Flags: needinfo?(botond)
We probably want to use Skia's built in macros for converting. SkScalar{Round,Ceil,Floor}ToInt(), depending on what behaviour we explicitly want.
Thanks Botond, so IIUC this is rounding towards zero.
The right thing to do here is likely to something like cairo's _cairo_fixed_from_double:

http://cgit.freedesktop.org/cairo/tree/src/cairo-fixed-private.h#n111

That way we can be more correct and have more speed.
Blocks: skia-reftest
I haven't tested this but it should be faster and more correct than previous patch and upstream skia code.
Duplicate of this bug: 996666
Assignee: nobody → jmuizelaar
This patch seems to cause assertion failures on some tests...
(In reply to Jeff Muizelaar [:jrmuizel] from comment #9)
> This patch seems to cause assertion failures on some tests...

Turns out these seemed to be caused by a conflict with the SkScalarToFDot6 macro in the same file.
I already fixed that issue locally and also on the graphics branch which I'm using to track progress: https://tbpl.mozilla.org/?tree=Graphics
Mike and I agreed on "SkScalarRoundToFDot6()"
Whiteboard: [skia-upstream]
Rubber stamp for importing this from upstream?
Attachment #8482803 - Flags: review?(jmuizelaar)
Attachment #8482803 - Flags: review?(jmuizelaar) → review+
Attachment #8482804 - Flags: review?(jmuizelaar) → review+
Oops, a bunch of my other patches got caught up in that.
https://hg.mozilla.org/mozilla-central/rev/ce39df5dccdb
https://hg.mozilla.org/mozilla-central/rev/c8d299030e1d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.