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

Unable to change text color in Three.js viewer #29228

Closed
jcamp0x2a mannequin opened this issue Feb 20, 2020 · 19 comments
Closed

Unable to change text color in Three.js viewer #29228

jcamp0x2a mannequin opened this issue Feb 20, 2020 · 19 comments

Comments

@jcamp0x2a
Copy link
Mannequin

jcamp0x2a mannequin commented Feb 20, 2020

The documentation for text3d suggests that the color of text can be adjusted:
https://doc.sagemath.org/html/en/reference/plot3d/sage/plot/plot3d/shapes2.html#sage.plot.plot3d.shapes2.text3d

But running the example provided results in only black text:

text3d("Sage is...",(2,12,1), color=(1,0,0)) + text3d("quite powerful!!",(4,10,0), color=(0,0,1))

Inspecting the threejs_template.html, in the addLabel function, the fill color is hard-coded:

context.fillStyle = 'black';

Encountered in 9.1.beta3 as well as 8.9.

Component: graphics

Keywords: threejs text color

Author: Paul Masson

Branch/Commit: 4ae1e77

Reviewer: Joshua Campbell

Issue created by migration from https://trac.sagemath.org/ticket/29228

@jcamp0x2a jcamp0x2a mannequin added this to the sage-9.1 milestone Feb 20, 2020
@jcamp0x2a jcamp0x2a mannequin added c: graphics labels Feb 20, 2020
@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Feb 21, 2020

comment:1

The only options currently passed in are text and location:

https://github.com/sagemath/sage-prod/blob/develop/src/sage/plot/plot3d/base.pyx#L422

Color can be added here and then processed in the addLabel function. Could include font size as well as an additional option.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Feb 28, 2020

comment:2

This commit passes text colors through to the template. Font size isn't being tracked by textures right now, and the entire graphics/keywords system is irritating and needs work, so passing font size should be left for another ticket.

The example above now shows the proper colors. One of the texts is clipped because the underlying Three.js sprite is limited in size. That can be addressed on another ticket as well, after some planned cleanup of sprite processing in the template.

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Feb 28, 2020

Author: Paul Masson

@paulmasson paulmasson mannequin added the s: needs review label Feb 28, 2020
@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Feb 28, 2020

Branch: u/paulmasson/29228

@jcamp0x2a
Copy link
Mannequin Author

jcamp0x2a mannequin commented Mar 2, 2020

Commit: 4056281

@jcamp0x2a
Copy link
Mannequin Author

jcamp0x2a mannequin commented Mar 2, 2020

comment:4

Hi Paul. I was testing out this branch and noticed that the colors weren't displaying in Chrome or Firefox. Looks like the # was missing at the beginning of the color string. Prepending it in base.pyx fixed it:

color = '#' + p.all[0].texture.hex_rgb()

I suppose it could also be prepended in the template right before it's used, depending on where you'd prefer to handle it.

Edit: somehow I seem to have changed the commit, though I don't understand how I was able to do that. Maybe because I was browsing the diff while writing this comment?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 2, 2020

Changed commit from 4056281 to 3a9c485

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 2, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

3a9c485Fix color

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Mar 2, 2020

Changed branch from u/paulmasson/29228 to u/paulmasson/29228-1

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Mar 2, 2020

comment:7

Joshua, the merge issue isn't your doing. The template moved from src/ext to src/sage/ext_data on #21785. Need to redo things...

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Mar 2, 2020

Changed commit from 3a9c485 to none

@paulmasson paulmasson mannequin added s: needs work and removed s: needs review labels Mar 2, 2020
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 3, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

4ae1e77Pass color through to texts

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 3, 2020

Commit: 4ae1e77

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Mar 3, 2020

Changed branch from u/paulmasson/29228-1 to u/paulmasson/29228

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Mar 3, 2020

comment:11

Joshua, should be set to go now. Please review!

@paulmasson paulmasson mannequin added s: needs review and removed s: needs work labels Mar 3, 2020
@jcamp0x2a
Copy link
Mannequin Author

jcamp0x2a mannequin commented Mar 3, 2020

comment:12

I've tested it again and the issue has been resolved. The correct text colors appear in the output. I made sure to update my start script to copy things from/to the new directories. I'll need to go into my animation branch and update things as well.

Thanks Paul!

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Mar 3, 2020

comment:13

Joshua, if you are satisfied with this ticket then enter your full name as a reviewer and set it to positive review.

@jcamp0x2a
Copy link
Mannequin Author

jcamp0x2a mannequin commented Mar 3, 2020

Reviewer: Joshua Campbell

@vbraun
Copy link
Member

vbraun commented Mar 5, 2020

Changed branch from u/paulmasson/29228 to 4ae1e77

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

No branches or pull requests

1 participant