Skip to content

Conversation

luisbg
Copy link
Contributor

@luisbg luisbg commented Feb 7, 2014

First stage of the optimizations changes.

Switched the data structure used for the set to a dynamic array of bit vectors like fontconfig's FcCharSet. Basic functions supported (creating, setting, checking, etc). A good moment to get it reviewed before switching the rest of the functions (algebra and iter).

Let me know when you have time to discuss this. I have a few ideas for further improvements in stage 2.
There are 2 smal bugs I am aware of, don't worry about them :)

hb_set_t previously used a statically allocated bit vector big enough to store
the whole range of codepoints. now, pages are created for each small bitvector
dynamically as needed.
@behdad
Copy link
Member

behdad commented Feb 10, 2014

Humm. Is github confused about this branch?

@luisbg
Copy link
Contributor Author

luisbg commented Feb 10, 2014

Very confused.

Ignore it until I finish this task. This branch was just for the review questions I asked last Thursday.

@behdad
Copy link
Member

behdad commented Feb 10, 2014

Ok. Just checked the patch. I think I have more comments for you. Wait for that before spending much more time on it. Thanks!

@@ -324,7 +324,7 @@ struct hb_set_t
static const unsigned int SHIFT = 5;
static const unsigned int BITS = (1 << SHIFT);
static const unsigned int MASK = BITS - 1;
static const unsigned int ELTS = (MAX_G + 1 + (BITS - 1)) / BITS;
static const unsigned int ELTS = (MAX_G + 1) / BITS;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, old cold was implementing ceil(), which is correct, since it does not make assumption on MAX_G + 1 being divisible by BITS.

@behdad
Copy link
Member

behdad commented Mar 25, 2017

Heh. I have had commented on that before... I'll try to clean this up and merge finally. :)

@luisbg
Copy link
Contributor Author

luisbg commented Mar 26, 2017

Would be nice to see this go in. It was my biggest contribution to harfbuzz :)

Let me know if I can help you merge this.

@luisbg
Copy link
Contributor Author

luisbg commented Apr 6, 2017

Hi Behdad.

Any news about this PR? Anything I can help you with?

Thanks 😄

@luisbg
Copy link
Contributor Author

luisbg commented Aug 25, 2017

@timofonic let's see what @behdad says 😄

@behdad behdad closed this in deed4a4 Oct 15, 2017
gpgreen pushed a commit to gpgreen/harfbuzz that referenced this pull request Jan 10, 2024
Add binding for kerning callbacks. Make feature fields public.
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.

2 participants