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.

Sign up for GitHub

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

Jump to bottom

Text cleanups #250

Merged
merged 13 commits into from
Apr 6, 2024
Merged

Text cleanups #250

merged 13 commits into from
Apr 6, 2024

Conversation

ubitux
Copy link
Contributor

@ubitux ubitux commented Apr 6, 2024

No description provided.

ubitux added 13 commits April 5, 2024 15:33
@ubitux
text: upload bg vertices as vec2 instead of vec3
1b568ea
@ubitux
text: replace char positioning mat4 with a vec4
9cfdc3f
@ubitux
text: simplify ratio scaling codepaths
05c92b8
@ubitux
text: move scale mode constants to header
c9a50be
@ubitux
text: move geometry computation and data buffers to the text unit
452b447 
Motivations for doing this move:
- it makes the node code only responsible for the pipeline and GPU data
  update code
- the logic of the text unit cannot be reused with a different geometry
  logic so it makes sense to have these computations together
- the CPU data allocation logic related to characters can be shared with
  the existing one currently used for effects
- having the geometry information available there will allow more
  advanced logic with regards to how we work with transforms in the
  future
@ubitux
text: simplify the error code path in refresh_geometry()
9c2c522
@ubitux
text: rename refresh_geometry() to refresh_pipeline_data()
f8ca3a9 
This function also refreshes the effects data buffers, this is not
limited to the geometry data.
@ubitux
text: rename text_effects_pointers struct to text_data_pointers
2f57784 
This structure now also contains the geometry.
@ubitux
text: split loop to make memory access contiguous
edf0dbf 
This is similar to what's being done in fill_default_data_buffers():
ptrs.* contains locations in the same buffer, so this prevents random
accesses.
@ubitux
text: remove a few unneeded includes
bca9826
@ubitux
text: fix and add a few whitespace cosmetics
e8b1ecc
@ubitux
text: use unsigned constants for flags
28ee204
@ubitux
text: use floats instead of double in {fg,bg}_color params
5ebca55
@ubitux ubitux requested a review from mbouron April 6, 2024 00:30
@mbouron
Copy link
Contributor

mbouron commented Apr 6, 2024 •
edited
Loading 

text: split loop to make memory access contiguous

This is similar to what's being done in fill_default_data_buffers():
ptrs.* contains locations in the same buffer, so this prevents random
accesses.

Did you measure any gain in practice ?

@ubitux
Copy link
Contributor Author

ubitux commented Apr 6, 2024 •
edited
Loading

Did you measure any gain in practice ?

No, nothing noticeable with my hardware, I'd probably have to use a large amount of data to spot this. It just makes the buffer reading completely linear since it has the following layout: [transform0][transform1][transform2]...[transformN][color0][color1][color2]...[colorN].... It's not too much change to split the loop so I'd rather do the sane thing here.

Edit: it would be a different story if we used a buffer of structs which would interlace the data

@ubitux ubitux merged commit 32ca6e3 into NopeForge:main Apr 6, 2024
15 checks passed
@ubitux ubitux deleted the text-cleanups branch April 6, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbouron mbouron Awaiting requested review from mbouron

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ubitux @mbouron