[HarfBuzz] HarfBuzz API design

[Carl Worth]

Excerpts from Behdad Esfahbod's message of Tue Aug 18 16:23:50 -0700 2009:
> [Warning: long email ahead]

My reply might bounce to unsubscribed-from lists. Feel free to forward
if you think there's anything important in here for others to read.

> _reference(), _destory(), and _get_reference_count().  At some point we may 
> want to add _[gs]et_user_data() also which is useful for language bindings.

That should be "destroy" not "destory", of course. I normally wouldn't
point out a typo in an email message, but I did see the same error in
a comment that might be copied from actual code:

> /* calls destory() when not needing user_data anymore */
> hb_face_t *
> hb_face_create_for_tables (hb_get_table_func_t  get_table,
>                             hb_destroy_func_t    destroy,
>                             void                *user_data);

So don't forget to spell-check your header files. :-)

> typedef enum {
>    HB_MEMORY_MODE_DUPLICATE,
>    HB_MEMORY_MODE_READONLY,
>    HB_MEMORY_MODE_WRITEABLE,
>    HB_MEMORY_MODE_READONLY_NEVER_DUPLICATE,
>    HB_MEMORY_MODE_READONLY_MAY_MAKE_WRITEABLE,
> } hb_memory_mode_t;
...
>    DUPLICATE: copy data right away and own it.
> 
>    READONLY: the data passed in can be kept for later use, but should not be 
> modified.  If modification is needed, the blob will duplicate the data lazily.
> 
>    WRITEABLE: data is writeable, use it freely.
> 
>    READONLY_NEVER_DUPLICATE: data is readonly and should never be duplicated. 
>   This disables operations needing write access to data.
> 
>    READONLY_MAY_MAKE_WRITEABLE: data is readonly but may be made writeable 
> using mprotect() or equivalent win32 calls.  It's up to the user to make sure 
> calling mprotect() or system-specific equivalents on the data is safe.  In 
> practice, that's never an issue on Linux and (according to Tor) on win32.

I don't think these names are quite right yet.

I think the thing that strikes me first as wrong is that if you create
a READONLY blob then it's still perfectly valid to write to it, (such
that you have to have a separate READONLY_NEVER_DUPLICATE for
"readonly---and I mean it).

One difficulty is that you're capturing separate notions here,
(whether the data buffer is writeable vs. whether the created blob
should be writeable).

And there's some missing orthogonality too. For example, if I have
data that I don't want the blob to reference, (hence DUPLICATE), might
I not still want to create a blob in which all operations requiring
write access are disabled? That combination is not possible to express
from what I can see.

So, here's pseudocode trying to capture the decision-tree for choosing
one of the modes currently:

if (the data cannot be referenced by the blob)
	DUPLICATE
else if (the data buffer can be written to)
	if (writing requires calling mprotect)
		READONLY_MAY_MAKE_WRITEABLE
	else
		WRITEABLE
else if (the resulting blob should be readonly)
	READONLY_NEVER_DUPLICATE
else
	READONLY;

So perhaps what's actually desired here is a set of flags to express
each of those conditions independently? I don't have a good proposal
for what those flag names might be.

Although, having a name for the entire behavior is helpful, (rather
than just a conjunction of flags). So maybe we just need simpler names
to capture the behaviors of interest:

COPY: Data is immediately copied by the blob. Resulting blob supports
	write operations. [New name for DUPLICATE]

COPY_ON_WRITE: Data is referenced by the blob. Resulting blob support
	write operations which are implemented by lazily copying the
	data, (the original data is never modified). [New name for
	READONLY]

READ_WRITE: Data is referenced by the blob. Resulting blob supports
	write operations which will directly modify original data.
	[New name for WRITEABLE. Of the new names I'm proposing, this
	is the one I like the least.]

READONLY: Data is referenced by the blob. Resulting blob does not
	support write operations. [New name for READONLY_NEVER_DUPLICATE]

MPROTECT: Data is referenced by the blob. Resulting blob supports
	write operations which are implemented by first calling
	mprotect or equivalent. [New name for
	READONLY_MAY_MAKE_WRITEABLE. Obviously this isn't an ideal
	name since it references an operating-system-specific
	feature. Suggestions welcome.]

I don't have much to say about the actual font-related parts of the
proposal. But I do think it's perhaps a bad idea to reuse the same
buffer for text input and glyph output. Is memory that tight? Might a
user not want to be able to query the text back out even after
shaping?

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/harfbuzz/attachments/20090819/679e2209/attachment.pgp>