Skip to content

Conversation

anood
Copy link
Contributor

@anood anood commented Nov 26, 2015

This function reorders all runs from logical to final visual order, similar to
fribidi_reorder_line() but without reordring the text inside the runs, so that
it can be used e.g. with HarfBuzz.

@anood anood changed the title add get_reorder_runs() API Add fribidi_reorder_runs() Nov 29, 2015
This function reorders all runs from logical to final visual order, similar to
fribidi_reorder_line() but without reordring the text inside the runs, so that
it can be used e.g. with HarfBuzz.
@behdad
Copy link
Collaborator

behdad commented Dec 10, 2015

Thanks. One problem I have with this patch is that it exposes FriBidiRun publicly. But I think if we are ever going to change that, we call it a new library anyway. I'm merging this.

Also, check out this algorithm that should be faster, but I never got to integrate in any codebase:
https://github.com/behdad/linear-reorder

behdad added a commit that referenced this pull request Dec 10, 2015
Add fribidi_reorder_runs()
@behdad behdad merged commit 9d8c69b into fribidi:master Dec 10, 2015
@khaledhosny
Copy link
Contributor

I’m not sure I get the library naming comment since this is now merged, are we changing the library name, bumping the soname version or what?

@behdad
Copy link
Collaborator

behdad commented Dec 10, 2015

I’m not sure I get the library naming comment since this is now merged, are we changing the library name, bumping the soname version or what?

No. I meant, if we ever want to change the structure of FriBidiRun, we can make that a different library or a soname bump.

@khaledhosny
Copy link
Contributor

I see, good then. I misread the original comment somehow.

@khaledhosny
Copy link
Contributor

Hmm, the unicode63 branch adds a new member to FriBidiRun, which breaks rebasing it on top of master. Also I don’t like that we don’t initialize all members of the returned structs in this function as they are unused here. I think it is better to have a different public struct for this function, and leave FriBidiRun alone? WDYT?

@behdad
Copy link
Collaborator

behdad commented Jan 13, 2016

I was actually thinking about this very same thing yesterday. Let me think about it.

@khaledhosny
Copy link
Contributor

Any updates?

@behdad
Copy link
Collaborator

behdad commented Jan 22, 2016

Hey. Sorry, busy with travel and working with the Hague people. So, I probably want to revert this, and do it in a slightly different way that doesn't expose the entire FriBidiRun, just the header. Will try to get it done by next week so we can move on with other changes.

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.

3 participants