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

Use Pathfinder for 2d canvas rendering #22957

Open
jdm opened this issue Mar 1, 2019 · 28 comments
Open

Use Pathfinder for 2d canvas rendering #22957

jdm opened this issue Mar 1, 2019 · 28 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Mar 1, 2019

Right now our 2d canvas rendering relies on a very old fork of Azure (https://github.com/servo/rust-azure/), which itself relies upon a very old fork of Skia (https://github.com/servo/rust-azure/). The operations that the 2d canvas allows are not complicated and it should be possible to use Pathfinder as the rendering backend instead. This would have several useful results:

  • it would remove multiple large C++ codebases from Servo's dependency tree
  • it would exercise Pathfinder in a real product
  • it would give us hardware acceleration instead of the current software rendering

I'm marking this as "hard" not because I actually believe it will be hard, per se, but it's going to require a bunch of investigation and poking and prodding to understand how to fit Pathfinder into Servo's canvas rendering pipeline. It could be fun, though!

@shanavas786
Copy link
Contributor

@shanavas786 shanavas786 commented Mar 6, 2019

@highfive assign me

@highfive
Copy link

@highfive highfive commented Mar 6, 2019

Hey @shanavas786! Thanks for your interest in working on this issue. It's now assigned to you!

@highfive highfive added the C-assigned label Mar 6, 2019
@shanavas786
Copy link
Contributor

@shanavas786 shanavas786 commented Mar 6, 2019

After adding pathfinder_gl = {git = "https://github.com/pcwalton/pathfinder", branch = "pf3"} to Cargo.toml for canvas, match build -d fails.

    Updating git repository `https://github.com/pcwalton/pathfinder`
    Updating crates.io index
   Compiling png v0.14.0
   Compiling gl v0.6.5
   Compiling mozjs_sys v0.61.6
   Compiling rust-webvr v0.10.2
error[E0599]: no function or associated item named `huffman_only` found for type `encoder::deflate::CompressionOptions` in the current scope
   --> /home/sinergia/.cargo/registry/src/github.com-1ecc6299db9ec823/png-0.14.0/src/common.rs:159:66
    |
159 |             Compression::Huffman => deflate::CompressionOptions::huffman_only(),
    |                                     -----------------------------^^^^^^^^^^^^
    |                                     |
    |                                     function or associated item not found in `encoder::deflate::CompressionOptions`

error[E0599]: no function or associated item named `rle` found for type `encoder::deflate::CompressionOptions` in the current scope
   --> /home/sinergia/.cargo/registry/src/github.com-1ecc6299db9ec823/png-0.14.0/src/common.rs:160:62
    |
160 |             Compression::Rle => deflate::CompressionOptions::rle(),
    |                                 -----------------------------^^^
    |                                 |
    |                                 function or associated item not found in `encoder::deflate::CompressionOptions`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0599`.
error: Could not compile `png`.
warning: build failed, waiting for other jobs to finish...

But cargo successfully builds pf3 branch in pathfinder repo with the same toolchain.

Am i missing something ?

@jdm
Copy link
Member Author

@jdm jdm commented Mar 6, 2019

Try runny ng cargo update -p png?

@shanavas786
Copy link
Contributor

@shanavas786 shanavas786 commented Mar 8, 2019

it didn't help

sinergia@sinergia:~/foss/servo$ cargo update -p png
error: There are multiple `png` packages in your project, and the specification `png` is ambiguous.
Please re-run this command with `-p <spec>` where `<spec>` is one of the following:
  png:0.12.0
  png:0.14.0
sinergia@sinergia:~/foss/servo$ cargo update -p png:0.12.0
    Updating crates.io index
sinergia@sinergia:~/foss/servo$ cargo update -p png:0.14.0
    Updating crates.io index
sinergia@sinergia:~/foss/servo$ ./mach build -d
   Compiling png v0.14.0
   Compiling mozjs_sys v0.61.6
error[E0599]: no function or associated item named `huffman_only` found for type `encoder::deflate::CompressionOptions` in the current scope
   --> /home/sinergia/.cargo/registry/src/github.com-1ecc6299db9ec823/png-0.14.0/src/common.rs:159:66
    |
159 |             Compression::Huffman => deflate::CompressionOptions::huffman_only(),
    |                                     -----------------------------^^^^^^^^^^^^
    |                                     |
    |                                     function or associated item not found in `encoder::deflate::CompressionOptions`

error[E0599]: no function or associated item named `rle` found for type `encoder::deflate::CompressionOptions` in the current scope
   --> /home/sinergia/.cargo/registry/src/github.com-1ecc6299db9ec823/png-0.14.0/src/common.rs:160:62
    |
160 |             Compression::Rle => deflate::CompressionOptions::rle(),
    |                                 -----------------------------^^^
    |                                 |
    |                                 function or associated item not found in `encoder::deflate::CompressionOptions`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0599`.
error: Could not compile `png`.
warning: build failed, waiting for other jobs to finish...
    Building [=================================================>     ] 547/594

@jdm
Copy link
Member Author

@jdm jdm commented Mar 11, 2019

Sorry for the delay. This is caused by a mistake in the png crate, where they rely on a feature from the deflate crate that was introduced in a version more recent than the one listed in their Cargo.toml. Running cargo update -p deflate fixes the problem.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 11, 2019

I filed image-rs/image-png#106 upstream to fix it.

@shanavas786
Copy link
Contributor

@shanavas786 shanavas786 commented Mar 12, 2019

👍

@shanavas786
Copy link
Contributor

@shanavas786 shanavas786 commented Mar 13, 2019

I think this is a bit heavier for me. It would be better if someone can mentor.

@krk
Copy link
Contributor

@krk krk commented Apr 23, 2019

Servo 54f54d1 can be built without errors with pathfinder_gl pcwalton/pathfinder@7cd05cd version added to components/canvas/Cargo.toml.

@jdm jdm removed the C-assigned label Apr 23, 2019
@krk
Copy link
Contributor

@krk krk commented Apr 27, 2019

I have created a demo project by duplicating the included demo for SVG and reusing it to render turtle graphics, maybe it is of any use here: https://github.com/krk/pathfinder/tree/pf3-turtle

image

Most of the turtle-renderer is copied directly from the svg-renderer demo.

@jdm
Copy link
Member Author

@jdm jdm commented May 3, 2019

@jdm
Copy link
Member Author

@jdm jdm commented May 3, 2019

There are four main parts to this work:

  • creating the GL context that pathfinder can use (we can possibly reuse the work to create a GL context for WebGL in this code)
  • creating a CanvasRenderingContext2D that can be used by all of the canvas operations
  • making the various canvas commands use pathfinder's canvas APIs
  • creating a pathfinder Renderer object that can be used when obtaining the canvas pixel data
@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 8, 2019

@highfive assign me

@highfive highfive added the C-assigned label May 8, 2019
@highfive
Copy link

@highfive highfive commented May 8, 2019

Hey @pylbrecht! Thanks for your interest in working on this issue. It's now assigned to you!

@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 8, 2019

Relevant code as stated by @jdm in #servo:

  • components/script/dom/canvas2drenderincontext.rs (I wouldn't expect many changes in this file)
  • components/canvas/canvas_paint_thread.rs
  • components/canvas/canvas_data.rs (the main canvas painting implementation)
@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 10, 2019

@jdm suggested https://github.com/jrmuizel/raqote might be another option. This way we just need to replace azure with raqote.

@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 13, 2019

After skimming through raqote and canvas_data.rs, I found some differences, which may or may not be important:

Please note that I did not dive deep into the code yet, just been looking at function names etc.

Although I'm very tempted to just start hacking and see where I end up, it might be more reasonable to identify crucial functionality first. So, what are absolute must-haves and what can we leave open to be implemented in the future?

@jdm
Copy link
Member Author

@jdm jdm commented May 13, 2019

I don't know that there are any particular must-haves. It's good to know that raqote has some missing pieces that will affect us. We should file issues about the path builder methods for sure; what does "more stuff" mean, exactly?

@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 13, 2019

what does "more stuff" mean, exactly?

At first glance: rect handling, stroke options, and shadows, but there could be more.

@jrmuizel
Copy link

@jrmuizel jrmuizel commented May 13, 2019

@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 20, 2019

Progress update:
The current approach as discussed with @jdm is to abstract the painting backend (azure) in such a way, that we can introduce another backend (raqote) step by step with successful builds in between.
We realize this by implementing traits for more complex azure members (e.g. azure_hl::DrawTarget) and wrapping trivial types in enums (e.g. azure_hl::Pattern).

Work done so far:

  • created GenericDrawTarget trait and implemented it for azure_hl::DrawTarget
  • created GenericPathBuilder trait and implemented it for azure_hl::PathBuilder
  • created enums for types, which came up by introducing the new traits

Due to life doing its thing (girlfriend back from vacation, moving to new appartment in two weeks, working on my thesis, etc.) my time is somewhat limited (again). I'm still very determined to keep working on this though, just at a slower pace, I guess.
Should I unexpectedly not be able to keep working on this, I'll definitely let you know, so someone else can get his/her hands on this.

@jdm
Copy link
Member Author

@jdm jdm commented May 20, 2019

Could you file a new issue tracking the raqote conversion? It would be good to keep this one focused on Pathfinder.

@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 20, 2019

Would implementing a generic backend fall under this issue or should I move this to the new issue, as well?

@pylbrecht pylbrecht mentioned this issue May 20, 2019
6 of 6 tasks complete
@cbrewster
Copy link
Member

@cbrewster cbrewster commented May 20, 2019

If you're planning on writing a generic backend for 2d graphics you may want to consider https://github.com/linebender/piet. Currently a Raqote backend is in the works and potentially a Pathfinder or Azure backend could be implemented

@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 20, 2019

So many options.. Honestly, I feel a bit lost of what solution to approach right now.
@jdm, what do you think?

@jdm
Copy link
Member Author

@jdm jdm commented May 20, 2019

We don't actually want to end up with a generic backend. It's a stop along the way to replacing our current backend with raqote, which is where we want to end up in the short term.

@pylbrecht
Copy link
Contributor

@pylbrecht pylbrecht commented May 20, 2019

@jdm, alright. So I continue replacing azure with raqote, but tracking my progress in #23431 instead of here. Then I guess you can unassign me here and assign me to #23431?

@jdm jdm removed the C-assigned label May 20, 2019
@utsavoza utsavoza mentioned this issue Jun 9, 2020
4 of 4 tasks complete
bors-servo added a commit that referenced this issue Jun 9, 2020
Implement CanvasRenderingContext2d.fillText

The PR consists of broadly two main changes:
- Implementation of Canvas2dRenderingContext.font
- Basic implementation of Canvas2dRenderingContext.fillText

Although I am not fully sure about the long term goals for the canvas backend in Servo, I assumed limited scope for font and text handling (should support simple text drawing with font selection) in the current implementation as I believe a more complete implementation would eventually be brought in as a part of #22957.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #11681
- [x] There are tests for these changes
bors-servo added a commit that referenced this issue Jun 12, 2020
Implement CanvasRenderingContext2d.fillText

The PR consists of broadly two main changes:
- Implementation of Canvas2dRenderingContext.font
- Basic implementation of Canvas2dRenderingContext.fillText

Although I am not fully sure about the long term goals for the canvas backend in Servo, I assumed limited scope for font and text handling (should support simple text drawing with font selection) in the current implementation as I believe a more complete implementation would eventually be brought in as a part of #22957.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #11681
- [x] There are tests for these changes
bors-servo added a commit that referenced this issue Jun 12, 2020
Implement CanvasRenderingContext2d.fillText

The PR consists of broadly two main changes:
- Implementation of Canvas2dRenderingContext.font
- Basic implementation of Canvas2dRenderingContext.fillText

Although I am not fully sure about the long term goals for the canvas backend in Servo, I assumed limited scope for font and text handling (should support simple text drawing with font selection) in the current implementation as I believe a more complete implementation would eventually be brought in as a part of #22957.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #11681
- [x] There are tests for these changes
bors-servo added a commit that referenced this issue Jun 12, 2020
Implement CanvasRenderingContext2d.fillText

The PR consists of broadly two main changes:
- Implementation of Canvas2dRenderingContext.font
- Basic implementation of Canvas2dRenderingContext.fillText

Although I am not fully sure about the long term goals for the canvas backend in Servo, I assumed limited scope for font and text handling (should support simple text drawing with font selection) in the current implementation as I believe a more complete implementation would eventually be brought in as a part of #22957.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #11681
- [x] There are tests for these changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.