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

Add hb_buffer_create_similar() #1555

Closed
behdad opened this issue Jan 24, 2019 · 11 comments · Fixed by #3353
Closed

Add hb_buffer_create_similar() #1555

behdad opened this issue Jan 24, 2019 · 11 comments · Fixed by #3353
Assignees

Comments

@behdad
Copy link
Member

behdad commented Jan 24, 2019

Right now one can create new buffer and append existing buffer to it, but in utils I also have this piece:

  static void copy_buffer_properties (hb_buffer_t *dst, hb_buffer_t *src) 
  { 
    hb_segment_properties_t props; 
    hb_buffer_get_segment_properties (src, &props); 
    hb_buffer_set_segment_properties (dst, &props); 
    hb_buffer_set_flags (dst, hb_buffer_get_flags (src)); 
    hb_buffer_set_cluster_level (dst, hb_buffer_get_cluster_level (src)); 
  } 
@ebraminio
Copy link
Collaborator

I needed this (and #1480) for some Kashida related testing thing, so.

@behdad
Copy link
Member Author

behdad commented Jan 24, 2019

Yes. Let's sort out the API first though. Should it create a new buffer? Or do we want a, essentially, assign version? I don't have a strong opinion.

@behdad
Copy link
Member Author

behdad commented Jan 24, 2019

The usecase in utils needs to copy just a range of the buffer. That's done using hb_buffer_append which takes a start/end. So, maybe we just need a function that copies the non-content properties like above. Just need to pick a name for it.

@ebraminio
Copy link
Collaborator

ebraminio commented Jan 24, 2019

I like something like Java's .clone() wording, hb_buffer_clone to return a new and modifiable buffer. My usecase was something like to copy a non-shaped buffer, shape it, maybe copy again and add Kashida glyph to places I like, and render it using hb-render, which I know, is superfluous usecase anyway and can be done completely without these and may is wrong layer to apply it, I however had no rush experimenting with the thing so.

@behdad
Copy link
Member Author

behdad commented Jan 25, 2019

Clone wouldn't address the need in util.

@behdad
Copy link
Member Author

behdad commented Jun 28, 2020

The usecase in utils needs to copy just a range of the buffer. That's done using hb_buffer_append which takes a start/end. So, maybe we just need a function that copies the non-content properties like above. Just need to pick a name for it.

What is also not address by hb_buffer_append is the effect on pre/post-context.

@behdad
Copy link
Member Author

behdad commented Jun 28, 2020

What is also not address by hb_buffer_append is the effect on pre/post-context.

And that's #1843

@behdad
Copy link
Member Author

behdad commented Jan 3, 2022

Getting ideas from cairo: hb_buffer_create_similar(). I like that.

@behdad
Copy link
Member Author

behdad commented Jan 3, 2022

hb_buffer_create_similar()

One problem with it: since, unfortunately, hb_buffer_clear_contents() also clears segment-properties, there's a need for two different two different functions. Maybe one doesn't need to be a function:

  • hb_buffer_create_similar() should copy everything other than contents, content-type, and segment-properties.

  • contents and content-type are handled by hb_buffer_append. Maybe segment-properties, if not set (individually, script, language, direction), should also be set by hb_buffer_append? Then the code in util will become automatic.

@behdad
Copy link
Member Author

behdad commented Jan 3, 2022

@khaledhosny wdyt? We already changed hb_buffer_append to take care of pre/post-context. I think taking care of segment-properties makes sense, since we consider segment-properties as part of content.

@behdad behdad changed the title Add hb_buffer_copy() Add hb_buffer_create_similar() Jan 3, 2022
@behdad behdad self-assigned this Jan 3, 2022
behdad added a commit that referenced this issue Jan 3, 2022
Now that hb_buffer_append() overlays properties, we can do this.

Part of #1555
behdad added a commit that referenced this issue Jan 3, 2022
Now that hb_buffer_append() overlays segment_properties we can do this.

Part of #1555
behdad added a commit that referenced this issue Jan 3, 2022
behdad added a commit that referenced this issue Jan 3, 2022
hb_buffer_reset() was NOT resetting cluster_level. Ouch! Fix that.

Part of #1555
behdad added a commit that referenced this issue Jan 3, 2022
matthiasclasen pushed a commit to matthiasclasen/harfbuzz that referenced this issue Jan 7, 2022
matthiasclasen pushed a commit to matthiasclasen/harfbuzz that referenced this issue Jan 7, 2022
Now that hb_buffer_append() overlays properties, we can do this.

Part of harfbuzz#1555
matthiasclasen pushed a commit to matthiasclasen/harfbuzz that referenced this issue Jan 7, 2022
Now that hb_buffer_append() overlays segment_properties we can do this.

Part of harfbuzz#1555
matthiasclasen pushed a commit to matthiasclasen/harfbuzz that referenced this issue Jan 7, 2022
matthiasclasen pushed a commit to matthiasclasen/harfbuzz that referenced this issue Jan 7, 2022
hb_buffer_reset() was NOT resetting cluster_level. Ouch! Fix that.

Part of harfbuzz#1555
matthiasclasen pushed a commit to matthiasclasen/harfbuzz that referenced this issue Jan 7, 2022
@khaledhosny
Copy link
Collaborator

@khaledhosny wdyt? We already changed hb_buffer_append to take care of pre/post-context. I think taking care of segment-properties makes sense, since we consider segment-properties as part of content.

Makes sense.

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 a pull request may close this issue.

3 participants