[PATCH v2] usp10: rewrite ScriptXtoCP
Huw Davies
huw at codeweavers.com
Fri Jan 15 07:46:21 CST 2016
On Fri, Jan 15, 2016 at 07:14:48AM -0600, Aric Stewart wrote:
> diff --git a/dlls/usp10/usp10.c b/dlls/usp10/usp10.c
> index f0bbafe..f6bfcbf 100644
> --- a/dlls/usp10/usp10.c
> +++ b/dlls/usp10/usp10.c
> +
> +/*
> + To handle multi-glyph clusters we need to find all the glyphs that are
> + represented in the cluster. This involves finding the glyph whose
> + index is the cluster index as well as whose glyph indices are greater than
> + our cluster index but not part of a new cluster.
> +
> + Then we sum all those glyphs' advances.
> +*/
> +static inline int get_cluster_advance(const int* piAdvance, const SCRIPT_VISATTR *psva, const WORD *pwLogClust, int cGlyphs, int cChars, int cluster, int direction)
I still think this line could do with wrapping.
> HRESULT WINAPI ScriptXtoCP(int iX,
> int cChars,
> @@ -2587,16 +2663,10 @@ HRESULT WINAPI ScriptXtoCP(int iX,
> int *piCP,
> int *piTrailing)
> {
> - int item;
> - float iPosX;
> - float iLastPosX;
> - int iSpecial = -1;
> - int iCluster = -1;
> - int clust_size = 1;
> - int cjump = 0;
> - int advance;
> - float special_size = 0.0;
> int direction = 1;
> + int iPosX;
> + int i;
> + int glyph_index, cluster_index;
>
> TRACE("(%d,%d,%d,%p,%p,%p,%p,%p,%p)\n",
> iX, cChars, cGlyphs, pwLogClust, psva, piAdvance,
> @@ -2605,6 +2675,7 @@ HRESULT WINAPI ScriptXtoCP(int iX,
> if (psa->fRTL && ! psa->fLogicalOrder)
> direction = -1;
>
> + /* Looking for non-reversed clusters in a reversed string */
> if (direction<0)
> {
> int max_clust = pwLogClust[0];
> @@ -2616,14 +2687,15 @@ HRESULT WINAPI ScriptXtoCP(int iX,
> return S_OK;
> }
>
> - for (item=0; item < cChars; item++)
> - if (pwLogClust[item] > max_clust)
> + for (i=0; i< cChars; i++)
> + if (pwLogClust[i] > max_clust)
> {
> ERR("We do not handle non reversed clusters properly\n");
> break;
> }
> }
>
> + /* Handle an iX < 0 */
> if (iX < 0)
> {
> *piCP = -1;
I know most of this code was already like this, but since we're
rewriting it anyway, I think it would be much clearer thus:
/* Handle an iX < 0 */
if (iX < 0)
{
if (direction < 0)
{
*piCP = cChars;
*piTrailing = 0;
}
else
{
*piCP = -1;
*piTrailing = 1;
}
return S_OK;
}
/* Looking for non-reversed clusters in a reversed string */
if (direction < 0)
{
for (i=0; i< cChars; i++)
if (pwLogClust[i] > max_clust)
{
FIXME("We do not handle non reversed clusters properly\n");
break;
}
}
(I also changed the ERR -> FIXME).
> @@ -2631,102 +2703,134 @@ HRESULT WINAPI ScriptXtoCP(int iX,
> return S_OK;
> }
>
> - iPosX = iLastPosX = 0;
> + /* find the glyph_index based in iX */
> if (direction > 0)
> - item = 0;
> + {
> + for (glyph_index = -1, iPosX = iX; iPosX >=0 && glyph_index < cGlyphs; iPosX -= piAdvance[glyph_index+1], glyph_index++)
> + ;
> + }
> else
> - item = cChars - 1;
> - for (; iPosX <= iX && item < cChars && item >= 0; item+=direction)
> - {
> - iLastPosX = iPosX;
> - if (iSpecial == -1 &&
> - (iCluster == -1 ||
> - (iCluster != -1 &&
> - ((direction > 0 && iCluster+clust_size <= item) ||
> - (direction < 0 && iCluster-clust_size >= item))
> - )
> - )
> - )
> + {
> + for (glyph_index = -1, iPosX = iX; iPosX > 0 && glyph_index < cGlyphs; iPosX -= piAdvance[glyph_index+1], glyph_index++)
> + ;
> + }
> +
> + TRACE("iPosX %i -> glyph_index %i (%i)\n", iPosX, glyph_index, cGlyphs);
> +
> + *piTrailing = 0;
> + if (glyph_index >= 0 && glyph_index < cGlyphs)
> + {
> + /* find the cluster */
> + if (direction > 0 )
> + for (i = 0, cluster_index = pwLogClust[0]; i < cChars && pwLogClust[i] <= glyph_index; cluster_index=pwLogClust[i++])
> + ;
> + else
> + for (i = 0, cluster_index = pwLogClust[0]; i < cChars && pwLogClust[i] >= glyph_index; cluster_index=pwLogClust[i++])
> + ;
> +
> + TRACE("cluster_index %i\n", cluster_index);
> +
> + if (direction < 0 && iPosX >= 0 && glyph_index != cluster_index)
> {
> - int check;
> - int clust = pwLogClust[item];
> + /* We are off the end of the string */
> + *piCP = -1;
> + *piTrailing = 1;
> + return S_OK;
> + }
>
> - iCluster = -1;
> - cjump = 0;
> - clust_size = get_cluster_size(pwLogClust, cChars, item, direction,
> - &iCluster, &check);
> - advance = get_glyph_cluster_advance(piAdvance, psva, pwLogClust, cGlyphs, cChars, clust, direction);
> + /* find the first character of the cluster */
> + for (i = 0; i < cChars && pwLogClust[i] != cluster_index; i++)
> + ;
I'm wondering whether instead of the above loop, you could modify get_cluster_charsize()
to return the start char as well as the size. Then you'd call it here and wouldn't
need to call it later on (in the else block below). That would save an extra loop
through the string.
>
> - if (check >= cChars && direction > 0)
> - {
> - int glyph;
> - for (glyph = clust; glyph < cGlyphs; glyph++)
> - special_size += get_glyph_cluster_advance(piAdvance, psva, pwLogClust, cGlyphs, cChars, glyph, direction);
> - iSpecial = item;
> - special_size /= (cChars - item);
> - iPosX += special_size;
> - }
> - else
> + TRACE("first char index %i\n",i);
> + if (scriptInformation[psa->eScript].props.fNeedsCaretInfo)
> + {
> + /* Check trailing */
> + if (glyph_index != cluster_index ||
> + (direction > 0 && abs(iPosX) <= (piAdvance[glyph_index] / 2)) ||
> + (direction < 0 && abs(iPosX) >= (piAdvance[glyph_index] / 2)))
> + *piTrailing = get_cluster_charsize(pwLogClust,cChars,cluster_index);
> + }
> + else
> + {
> + int cluster_size;
> +
> + cluster_size = get_cluster_charsize(pwLogClust, cChars, cluster_index);
> +
> + if (cluster_size > 1)
> {
> - if (scriptInformation[psa->eScript].props.fNeedsCaretInfo)
> + /* Be part way through the glyph cluster based on size and position */
> + int cluster_advance = get_cluster_advance(piAdvance, psva, pwLogClust, cGlyphs, cChars, cluster_index, direction);
> + double cluster_part_width = cluster_advance / (float)cluster_size;
> + double adv;
> + int part_index;
> +
> + /* back up to the beginning of the cluster */
> + for (adv = iPosX, part_index = cluster_index; part_index <= glyph_index; part_index++)
> + adv += piAdvance[part_index];
> + if (adv > iX) adv = iX;
> +
> + TRACE("Multi-char cluster, no snap\n");
> + TRACE("cluster size %i, pre-cluster iPosX %f\n",cluster_size, adv);
> + TRACE("advance %i divides into %f per char\n", cluster_advance, cluster_part_width);
> + if (direction > 0)
> {
> - if (!cjump)
> - iPosX += advance;
> - cjump++;
> + for (part_index = 0; adv >= 0; adv-=cluster_part_width, part_index++)
> + ;
> + if (part_index) part_index--;
> }
> else
> - iPosX += advance / (float)clust_size;
> + {
> + for (part_index = 0; adv > 0; adv-=cluster_part_width, part_index++)
> + ;
> + if (part_index > cluster_size)
> + {
> + adv += cluster_part_width;
> + part_index=cluster_size;
> + }
> + }
> +
> + TRACE("base_char %i part_index %i, leftover advance %f\n",i, part_index, adv);
> +
> + if (direction > 0)
> + i += part_index;
> + else
> + i += (cluster_size - part_index);
> +
> + /* Check trailing */
> + if ((direction > 0 && fabs(adv) <= (cluster_part_width / 2.0)) ||
> + (direction < 0 && adv && fabs(adv) >= (cluster_part_width / 2.0)))
> + *piTrailing = 1;
> }
> - }
> - else if (iSpecial != -1)
> - iPosX += special_size;
> - else /* (iCluster != -1) */
> - {
> - int adv = get_glyph_cluster_advance(piAdvance, psva, pwLogClust, cGlyphs, cChars, pwLogClust[iCluster], direction);
> - if (scriptInformation[psa->eScript].props.fNeedsCaretInfo)
> + else
> {
> - if (!cjump)
> - iPosX += adv;
> - cjump++;
> + /* Check trailing */
> + if ((direction > 0 && abs(iPosX) <= (piAdvance[glyph_index] / 2)) ||
> + (direction < 0 && abs(iPosX) >= (piAdvance[glyph_index] / 2)))
> + *piTrailing = 1;
> }
> - else
> - iPosX += adv / (float)clust_size;
> - }
> - }
> -
> - if (direction > 0)
> - {
> - if (iPosX > iX)
> - item--;
> - if (item < cChars && ((iPosX - iLastPosX) / 2.0) + iX >= iPosX)
> - {
> - if (scriptInformation[psa->eScript].props.fNeedsCaretInfo && clust_size > 1)
> - item+=(clust_size-1);
> - *piTrailing = 1;
> }
> - else
> - *piTrailing = 0;
> }
> else
> {
> - if (iX == iLastPosX)
> - item++;
> - if (iX >= iLastPosX && iX <= iPosX)
> - item++;
> + TRACE("Point falls outside of string\n");
> + if (glyph_index < 0)
> + i = cChars-1;
> + else /* (glyph_index >= cGlyphs) */
> + i = cChars;
>
> - if (iLastPosX == iX)
> - *piTrailing = 0;
> - else if (item < 0 || ((iLastPosX - iPosX) / 2.0) + iX <= iLastPosX)
> + /* If not snaping in the reverse direction (such as Hebrew) Then 0
> + point flow to the next character */
> + if (direction < 0)
> {
> - if (scriptInformation[psa->eScript].props.fNeedsCaretInfo && clust_size > 1)
> - item-=(clust_size-1);
> - *piTrailing = 1;
> + if (!scriptInformation[psa->eScript].props.fNeedsCaretInfo && abs(iPosX) == piAdvance[glyph_index])
> + i++;
> + else
> + *piTrailing = 1;
> }
> - else
> - *piTrailing = 0;
> }
>
> - *piCP = item;
> + *piCP = i;
>
> TRACE("*piCP=%d\n", *piCP);
> TRACE("*piTrailing=%d\n", *piTrailing);
>
>
More information about the wine-devel
mailing list