Skip to content

DM-7765 Render the text defined as the 'text' property value of a region on PNG #195

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

Merged
merged 2 commits into from
Sep 30, 2016

Conversation

cwang2016
Copy link
Contributor

The rendering of the region associated text was missing in the PNG.
This issue is fixed by creating a separate text region with the location calculated at JS parts, the text value and font defined in the original region, and sending the created text regions as well as the original regions to the server for producing the PNG.

@cwang2016
Copy link
Contributor Author

Test sample:
firefly_test_data/region-test-files/textText_DM-7765.reg

Copy link
Contributor

@robyww robyww left a comment

Choose a reason for hiding this comment

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

Looks good. Someday we need to make the java produce something that is closer to the client overlay. However, that is out of scope for the ticket and may involve a look of work.

Good Job.

* @param name string - ex: Arieal, Courier, Ties, Helvetica, sans-serif, BRAGGADOCIO
* @param point string (font size) ex: 10, 12, 14, 16
* @param weight string ex: normal, bold
* @param slant string ex: normal, italic
Copy link
Contributor

Choose a reason for hiding this comment

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

I know the jsdocs were wrong before bug it you add to them you should go ahead and fix them.
Such as:
@param {string} slant ex: normal, italic

@cwang2016 cwang2016 merged commit 37199e0 into dev Sep 30, 2016
@cwang2016 cwang2016 deleted the DM-7765-PNGText branch September 30, 2016 01:11
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.

2 participants