-
Notifications
You must be signed in to change notification settings - Fork 152
Canvas plugin #1468
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
base: master
Are you sure you want to change the base?
Canvas plugin #1468
Conversation
Adds implementation for canvas `transform` function https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/transform
Add `fontBoundingBoxAscent` and `fontBoundingBoxDescent` to textMetrics
- Adds bindings for setting and getting `letterSpacing` https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/letterSpacing - Adds implementation for setter and getter
…nto CanvasTest
Co-authored-by: Ryan Tremblay <[email protected]>
- `lineCap` https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/lineCap - `lineJoin` https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/lineJoin - `miterLimit` https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/miterLimit --------- Co-authored-by: Cedric Guillemet <[email protected]>
Gradient / color stop glue code and small example. 
… into CanvasTest # Conflicts: # Polyfills/Canvas/Source/Context.cpp
- https://developer.mozilla.org/en-US/docs/Web/API/Path2D Adds nanosvg.h for SVG path parsing. Known missing functionality: 1. `addPath` doesn't accept DOMMatrix transform 2. `roundRect` doesn't accept CSS-style radii array 3. `ellipse` doesn't handle clockwise Called from BabylonJS/Babylon.js#16221
…nto CanvasTest
- https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/getTransform - https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/setTransform - https://developer.mozilla.org/en-US/docs/Web/API/Path2D/addPath#transform usage: ``` // Path 2D let path = new _native.Path2D(); let path2 = new _native.Path2D("M 10,30 A 20, 20 0, 0, 1 50, 30 A 20, 20 0, 0, 1 90, 30 Q 90, 60 50, 90 Q 10, 60 10, 30 z"); path.addPath(path2, { a: 1, b: 0, c: 0, d: 1, e: 400, f: 0 }); // shift right 400 context.stroke(path); ```
…nto CanvasTest
… into CanvasTest
https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/roundRect Known Issues: - clipping round rect does not have rounded corners Co-authored-by: Cedric Guillemet <[email protected]>
Small PR just so later PRs that require modifying nanovg have proper diffs
- allow NVGPaint to have 2 images (onlye used when text is mixed with gradient) - shader update 
I'll need someone to generate the HSSL shader for me since I'm on Mac. Currently doesn't support blurred text but can add that in a future PR. --------- Co-authored-by: Cedric Guillemet <[email protected]>
- Adds example Path2D usage in experience.js. - Makes Path2D work with .fill as well  --------- Co-authored-by: Cedric Guillemet <[email protected]>
Fixes: - Depth stencil missing. Not optional in nanovg - Unnecessary texture creation. This was caused by a bad merge resolution ([link](50697ff)) - Missing `m_available` increment when framebuffer released caused infinite framebuffer creation The various warnings about missing stencil bugger and failed to allocate framebuffer are now gone.
MDN: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/roundRect Implements CSS-style radius array argument 
MDN: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/direction MDN: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/getTransform - Context direction is incomplete. Further investigation is needed on a solution to deal with ligatures (eg. stb_freetype + harfbuzz?) - Context getTransform() now returns missing DOMMatrix properties
…nto CanvasTest # Conflicts: # Polyfills/Canvas/Source/Canvas.cpp # Polyfills/Canvas/Source/Canvas.h # Polyfills/Canvas/Source/Context.cpp # Polyfills/Canvas/Source/Context.h
W3: https://www.w3.org/TR/SVG11/filters.html#feGaussianBlurElement - 13-tap gaussian blur for s < 2 - 3-pass box blur for s >= 2 Notes: - gaussian weights no longer hardcoded. we can optimize by caching weights. - box blur handles even vs odd kernel as per W3. sanity limit of 1000px. - edge sampling set to mirror. this avoids artifacts when blurring close to edge. Testing with even-case box blur d=8: `ctx.filter='blur(4px)'` 
MDN: https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/roundRect Adds support for single DOMPoint argument in roundRect (Context, Path2D) Looks like `nvgRoundedRectVarying` already implemented seperate x/y radius, so just needed to add extra args 
@ryantrem found that when we load in a replacement font buffer.. the old buffer passed to nvgCreateFontMem is freed and can get corrupted To solve this, we've made LoadTTFAsync effectively synchronous and don't allow loading in a duplicate font. We can consider future work to support safely updating existing fonts
small followup to #1516
…nto CanvasTest # Conflicts: # .github/jobs/test_install_win32.yml # Apps/UnitTests/Scripts/tests.js # Plugins/NativeEngine/CMakeLists.txt
Comparison PG : https://playground.babylonjs.com/#TKVFSA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a lot of this has already been reviewed in the side branch, so mostly just commenting on some high level things related to bringing the code into master.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a readme.md explaining where this OSS code came from and whether we have modified it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And also how nanovg_babylon.* relates to the other files?
Polyfills/Canvas/Source/Context.cpp
Outdated
#include "Gradient.h" | ||
|
||
/* | ||
Most of these context methods are preliminary work. They are currenbly not tested properly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still want this comment? Also there is a typo in it anyway (currenbly).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, it's better to add a readme and document the plugin as experimental IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a readme.md explaining where this came from and if it has any Babylon modifications also?
PRIVATE GraphicsDeviceContext | ||
PRIVATE JsRuntime | ||
PRIVATE SPIRV) | ||
if(BGFX_BUILD_TOOLS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this more? What's this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bgfx shaderc tool needed to be compiled to apply changes in nanovg shaders. But, BN and bgfx need different versions spirv/glslang with different dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I still don't understand. What does bgfx's shaderc tool have to do with NativeEngine?
static constexpr int NORMAL_WEIGHT = 400; | ||
static constexpr int BOLD_WEIGHT = 700; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static constexpr int NORMAL_WEIGHT = 400; | |
static constexpr int BOLD_WEIGHT = 700; | |
static constexpr const int NORMAL_WEIGHT = 400; | |
static constexpr const int BOLD_WEIGHT = 700; |
FontStyle style{FontStyle::Normal}; | ||
int weight{NORMAL_WEIGHT}; | ||
float size{10}; | ||
std::string family{"sans-serif"}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These variables should have m_
prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even public? I commented on them being public, so if we switch and have get functions, then it seems like they should be m_
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this PR made all of these variables private?
float GetSize() const { return size; } | ||
const std::string& GetFamiliy() const { return family; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Spelling, though I would recommend we do it like this.
float GetSize() const { return size; } | |
const std::string& GetFamiliy() const { return family; } | |
float Size() const { return size; } | |
const std::string& Family() const { return family; } |
Uh oh!
There was an error while loading. Please reload this page.