Allow create_text to accept multiple texts and materials fixes #1012#1101
Allow create_text to accept multiple texts and materials fixes #1012#1101AdityaGupta716 wants to merge 16 commits intofury-gl:v2from
Conversation
|
@maharshi-gor I've implemented the changes for this issue. Plz review |
| The material object or list of material objects. | ||
| **kwargs : dict | ||
| Additional properties like font_size, anchor, etc. | ||
|
|
There was a problem hiding this comment.
Please also think about how do you provide all the other keyword arguments in the case of multiple text?
Will the position be same for all of them?
|
Hi @AdityaGupta716 thanks for the work. Please take a look at the repository structure before submitting a PR. |
Updated create_text function to support per-text customization for materials and additional kwargs.
|
@maharshi-gor, the kwargs are fully flexible — pass a single value and it broadcasts to all text objects, or pass a list for per-text control. For example font_size=24 applies to all, while position=[(0,0,0), (1,0,0)] sets each independently. Length mismatches raise a ValueError immediately. Let me know if you need anything else. |
maharshi-gor
left a comment
There was a problem hiding this comment.
As I suspect the PR needs to update different file and add the test cases at different location according to that.
| >>> # Multiple texts, shared material and kwargs | ||
| >>> t = create_text(["Hello", "World", "FURY"], mat) | ||
|
|
||
| >>> # Multiple texts with individual materials and per-text positions |
There was a problem hiding this comment.
Remove comments from examples.
| def create_text(text, material, **kwargs): | ||
| """Create a text object. | ||
| """Create a text object or a group of text objects. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| text : str | ||
| The text content. | ||
| material : TextMaterial | ||
| The material object. | ||
| text : str or list of str | ||
| The text content or list of text contents. | ||
| material : TextMaterial or list of TextMaterial | ||
| The material object or list of material objects. | ||
| **kwargs : dict | ||
| Additional properties like font_size, anchor, etc. | ||
| Additional properties like font_size, anchor, position, etc. | ||
| When ``text`` is a list, each kwarg can be either: | ||
| - A single value applied to all text objects, or | ||
| - A list of values (one per text) for per-text customization. | ||
|
|
||
| Returns | ||
| ------- | ||
| Text | ||
| The text object. | ||
| Text or Group | ||
| A single Text object if text is a string, or a Group of Text | ||
| objects if text is a list. | ||
|
|
||
| Raises | ||
| ------ | ||
| TypeError | ||
| If text is not a string or material is not an instance of TextMaterial. | ||
| If text is not a string or list of strings, or material is not | ||
| an instance of TextMaterial or list of TextMaterial. | ||
| ValueError | ||
| If the length of material list or any kwarg list does not match | ||
| the length of the text list. | ||
|
|
||
| Examples | ||
| -------- | ||
| >>> mat = TextMaterial() | ||
| >>> # Single text (unchanged behavior) | ||
| >>> t = create_text("Hello", mat) | ||
|
|
||
| >>> # Multiple texts, shared material and kwargs | ||
| >>> t = create_text(["Hello", "World", "FURY"], mat) | ||
|
|
||
| >>> # Multiple texts with individual materials and per-text positions | ||
| >>> t = create_text( | ||
| ... ["Hello", "World"], | ||
| ... [TextMaterial(), TextMaterial()], | ||
| ... position=[(0, 0, 0), (1, 0, 0)], | ||
| ... ) | ||
| """ | ||
| if isinstance(text, list): | ||
| if not all(isinstance(t, str) for t in text): | ||
| raise TypeError("All items in text list must be strings.") | ||
|
|
||
| n = len(text) | ||
|
|
||
| if isinstance(material, list): | ||
| if len(material) != n: | ||
| raise ValueError( | ||
| "Length of material list must match length of text list." | ||
| ) | ||
| if not all(isinstance(m, TextMaterial) for m in material): | ||
| raise TypeError( | ||
| "All items in material list must be instances of TextMaterial." | ||
| ) | ||
| materials = material | ||
| else: | ||
| if not isinstance(material, TextMaterial): | ||
| raise TypeError("material must be an instance of TextMaterial.") | ||
| materials = [material] * n | ||
|
|
||
| # Normalize kwargs: expand scalars to per-text lists | ||
| norm_kwargs = {} | ||
| for k, v in kwargs.items(): | ||
| if isinstance(v, list): | ||
| if len(v) != n: | ||
| raise ValueError( | ||
| f"Length of kwarg '{k}' list must match length of text list." | ||
| ) | ||
| norm_kwargs[k] = v | ||
| else: | ||
| norm_kwargs[k] = [v] * n | ||
|
|
||
| group = Group() | ||
| for i, (t, m) in enumerate(zip(text, materials, strict=True)): | ||
| per_text_kwargs = {k: norm_kwargs[k][i] for k in norm_kwargs} | ||
| position = per_text_kwargs.pop("position", None) | ||
| actor = Text(text=t, material=m, **per_text_kwargs) | ||
| if position is not None: | ||
| actor.local.position = position | ||
| group.add(actor) | ||
| return group | ||
|
|
||
| if not isinstance(text, str): | ||
| raise TypeError("text must be a string.") | ||
|
|
||
| if not isinstance(material, TextMaterial): | ||
| raise TypeError("material must be an instance of TextMaterial.") | ||
|
|
||
| text = Text(text=text, material=material, **kwargs) | ||
| return text | ||
| return Text(text=text, material=material, **kwargs) |
There was a problem hiding this comment.
I feel this should not be done here. Please check the text method in the planar.py
Line 497 in 8bb525c
This method should be updated to accept multiple text not this method. The text method is the one which people should use.
Fixes #1012
Changes
create_textinfury/actor/core.pyto accept a list ofstrings and a list of materials
Textactor for a string (backward compatible)GroupofTextactors for a list of stringsfury/actor/tests/test_actor_text.pyUsage