Hello, 
I could send you the patch whenever you give the green signal.

Aamir

I don't see anything that blocks submitting a patch.

As Owen said, nothing blocks you from submitting a patch.  But I'm wondering
what do you really mean by this.  Using Nafees Nastaliq font myself, I see that
it's already possible to do what you explained.  Nafees font can be found in bug
122330.  See this screenshot for what Pango can currently do (the upper one):

http://bugzilla.gnome.org/attachment.cgi?id=23841&action=view

Created attachment 30384 [details] [review]
implements reverse chain subst. table

Well its a long story. You see when we first made this font, this 'type 8 
luxury' was new and unavailable (at least in VOLT at that time). So we had to 
model the script using the simple chain table. As a result instead of one OT 
rule we had to write (in some cases upto) 5 to get that output. If you are 
using the font you will see that its pretty slow. Work is still going on on 
this font to speed it up a little. We are actually in the design phase 
of 'refurnishing' the font where we are thinking about reducing the # of rules 
without affecting the output. We have other ideas about the huge number of 
positioning rules but as far as subs. rules are concerned our initial study 
shows that by simple lookup ordering, grouping and intrducing type8 we can 
reduce the number of GSUB lookups. Its going to take time but in the meantime  
we had this time to work in pango and get the type 8 table moving.

Thanks for using our font. You can try our other fonts at www.crulp.org

Only recently the biggest Urdu newspaper of pakistan (or the world) choose to 
use one of our other font for their web site. http://www.jang.net/urdu.

Bye 

Aamir

If you look at how the lookup type 8 is defined, it's carefully
restricted so that it can be done in-place. It's always a one-to-one
substitution.

otl_buffer_add_output_glyph_reverse() has all the inefficiency of
what we do for the normal case *and* it only works for 1-to-1
substitutions.

I think it would be better to write another loop that replaces
Do_String_Lookup() for the particular case of lookup 8 and
instead of copying in=>out, just modify in in place. (Look-behind
refers to the modified glyphs, so this works fine)

 FT_ULong i;

 if (buffer->in_length == 0)
   return;

 i = buffer->in_length - 1;
 do
 {
    if ( ~IN_PROPERTIES( i ) & properties[lookup_index] )
      Lookup_ReverseChainContextSubst ( ... )
 
 }
 while (i--)

You get the idea. (Careful with reverse iteration for an unsigned index variable);

I have a few over heads in my code. For e.g. i had been optimistic that in 
future there may be more than one format for reverse chain. Any ways this seems 
fine. Do i have to send you a new patch or will this do.

I'd appreciate a new patch.

Created attachment 30914 [details] [review]
Corrected patch

Here's a revised version of your patch ... can you test?

Summary of changes:

 - The code only checked the properties for the first character
   in the buffer
 - Apply_ReverseChainContextSubst was returning TTO_Err_Not_Covered
   in most cases, needed to copy the retError handling from 
   Do_String_Lookup
 - Check for GSUB_LOOKUP_REVERSE_CHAIN in Do_Glyph_Lookup and
   return an error. (See also bug 161327)
 - Removed unused context_length and nesting_level
   arguments to Lookup_ReverseChainContextSubst
 - Fix off by one in
       if ( bgc > string_index || string_index + lgc > buffer->in_length )
   need to account for 1 input glpyh
 - Fixed usage of IN_CURGLYPH(0) in Lookup_ReverseChainContextSubst,
   should be IN_ITEM( string_index ) (GLYPH=>ITEM change probably
   postdates this patch)
 - Fixed usage of buffer->out_pos in Lookup_ReverseChainContextSubst
 - Used various variables to simplify TT_GSUB_Apply_String changes
 - Some formatting, comment fixes to make it conform to existing code,
   fix variable declaration mixed with code.
 - Fixed leak of rccs->Substitute in Free_ReverseChainContextSubst
 - Fixed leaks of rccs-Coverage in Load_ReverseChainContextSubst (spurious 
   return, return instead of goto Fail4)
 - Check that SubstFormat == 1.
 - Make Apply_ReverseChainContextSubst const

I haven't tested beyond compilation

Created attachment 34842 [details] [review]
Version with my fixes

Hello, 

Certainly. Can you please send me the (.zip and .rar) version of OpenType 
Folder. Our download services have currently been suspended. I will need the 
updated version because there is no use of IN_ITEM in the one i have... and i 
cant download either.

Aamir

Hello, 

Certainly. Can you please send me the (.zip and .rar) version of OpenType 
Folder. Our download services have currently been suspended. I will need the 
updated version because there is no use of IN_ITEM in the one i have... and i 
cant download either.

Aamir

Dear Owen,

I have very carefully tested these changes and everything seem fine. There is 
this silly error though. The function call to otl_buffer_copy_output_glyph ( 
in do_string_lookup) has a space between the function name and opening bracket.
No need discussing this..anyway, is there anything else you would want me to 
do?

Aamir

Hi,

Any plans of incorporating GSUB 8 in the next release?

Aamir

I'm working on a unified OpenType code base.  I will include your patch.  That
should not take more than a couple months to show up in the release.

The patch needs some slight changes to apply on the current code base.  An updated patch will be committed instantly.

Created attachment 58430 [details] [review]
committed patch.

2006-01-30  Behdad Esfahbod  <behdad@gnome.org>

        * pango/opentype/ftxgsub.c: Handle GSUB Lookup type 8,
        and ReverseChainContextualSubst table.  (bug #149696,
        patch from Aamir Wali)

Thanks for the patch.  It's in CVS and will be in Pango 1.11.3 due to release in an hour.  Please test.

Closing as it's fixed.  Test please.

Aamir, did you ever update the Nafees Nastaliq font to use this type of lookups?  I'm looking for any font using it and fail to find any.  Thanks.