<div dir="ltr">Thanks for review!<div>I sent an improved series.</div></div><div class="gmail_extra"><br><div class="gmail_quote">2018-04-12 16:47 GMT+08:00 Huw Davies <span dir="ltr"><<a href="mailto:huw@codeweavers.com" target="_blank">huw@codeweavers.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, Apr 12, 2018 at 11:31:29AM +0800, Jactry Zeng wrote:<br>
> Signed-off-by: Jactry Zeng <<a href="mailto:jzeng@codeweavers.com">jzeng@codeweavers.com</a>><br>
> ---<br>
>  dlls/riched20/caret.c   | 30 ++++++++++++++++++++++++++++--<br>
>  dlls/riched20/editor.c  |  1 +<br>
>  dlls/riched20/editstr.h |  8 +++++++-<br>
>  dlls/riched20/list.c    |  8 ++++++--<br>
>  dlls/riched20/richole.c | 20 ++++++++++----------<br>
>  dlls/riched20/run.c     |  2 +-<br>
>  dlls/riched20/writer.c  |  6 +++---<br>
>  7 files changed, 56 insertions(+), 19 deletions(-)<br>
> <br>
> <br>
<br>
</div></div>> diff --git a/dlls/riched20/caret.c b/dlls/riched20/caret.c<br>
> index d676a1bc38..1e654a0a80 100644<br>
> --- a/dlls/riched20/caret.c<br>
> +++ b/dlls/riched20/caret.c<br>
> @@ -471,6 +471,14 @@ ME_<wbr>InternalInsertTextFromCursor(<wbr>ME_TextEditor *editor, int nCursor,<br>
>    return ME_InsertRunAtCursor(editor, p, style, str, len, flags);<br>
>  }<br>
>  <br>
> +static ME_ReObjItem* create_reobj_item(const REOBJECT* reo)<br>
> +{<br>
> +    ME_ReObjItem *reo_item = heap_alloc(sizeof(ME_<wbr>ReObjItem));<br>
<br>
  sizeof(*reo_item) like the one you do on the next line.<br>
<br>
Failure test.<br>
<br>
> +<br>
> +    reo_item->reobj = heap_alloc(sizeof(*reo));<br>
<br>
Having two allocs here is needless.  embed the REOBJECT structure<br>
inside ME_ReObjItem.<br>
<br>
> +    ME_CopyReObject(reo_item-><wbr>reobj, reo);<br>
> +    return reo_item;<br>
> +}<br>
>  <br>
>  void ME_InsertOLEFromCursor(ME_<wbr>TextEditor *editor, const REOBJECT* reo, int nCursor)<br>
>  {<br>
> @@ -484,8 +492,26 @@ void ME_InsertOLEFromCursor(ME_<wbr>TextEditor *editor, const REOBJECT* reo, int nCur<br>
>  <br>
>    di = ME_<wbr>InternalInsertTextFromCursor(<wbr>editor, nCursor, &space, 1, pStyle,<br>
>                                         MERF_GRAPHICS);<br>
> -  di->member.run.ole_obj = heap_alloc(sizeof(*reo));<br>
> -  ME_CopyReObject(di->member.<wbr>run.ole_obj, reo);<br>
> +  di->member.run.reo_item = create_reobj_item(reo);<br>
> +<br>
> +  if (list_empty(&editor->reobj_<wbr>list))<br>
> +      list_add_head(&editor->reobj_<wbr>list, &di->member.run.reo_item-><wbr>entry);<br>
<br>
I don't think this is a useful optimization.  It's probably also hiding<br>
a bug below.<br>
<br>
> +  else<br>
> +  {<br>
> +      ME_DisplayItem *di_prev = di;<br>
> +      ME_ReObjItem *reo_item_prev = NULL;<br>
> +<br>
> +      while (ME_PrevRun(NULL, &di_prev, TRUE))<br>
> +      {<br>
> +          if (di_prev->member.run.reo_item)<br>
> +          {<br>
> +              reo_item_prev = di_prev->member.run.reo_item;<br>
> +              break;<br>
> +          }<br>
> +      }<br>
<br>
At this point reo_item_prev could be NULL, if it would be the first<br>
item in the list.<br>
<br>
> +      list_add_after(&reo_item_prev-<wbr>>entry, &di->member.run.reo_item-><wbr>entry);<br>
> +  }<br>
> +<br>
>    ME_ReleaseStyle(pStyle);<br>
>  }<br>
>  <br>
> diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c<br>
> index c4425127e7..c57d3ea9b9 100644<br>
> --- a/dlls/riched20/editor.c<br>
> +++ b/dlls/riched20/editor.c<br>
> @@ -3113,6 +3113,7 @@ ME_TextEditor *ME_MakeEditor(ITextHost *texthost, BOOL bEmulateVersion10)<br>
>    ed->wheel_remain = 0;<br>
>  <br>
>    list_init( &ed->style_list );<br>
> +  list_init( &ed->reobj_list );<br>
>    OleInitialize(NULL);<br>
>  <br>
>    return ed;<br>
> diff --git a/dlls/riched20/editstr.h b/dlls/riched20/editstr.h<br>
> index a37c4de46e..b32c2622c6 100644<br>
> --- a/dlls/riched20/editstr.h<br>
> +++ b/dlls/riched20/editstr.h<br>
> @@ -153,6 +153,11 @@ typedef enum {<br>
>  <br>
>  struct tagME_DisplayItem;<br>
>  <br>
> +typedef struct tagME_ReObjItem {<br>
> +    struct list entry;<br>
> +    REOBJECT *reobj;<br>
<br>
As already suggested this become REOBJECT reobj<br>
<br>
> +} ME_ReObjItem;<br>
<br>
Let's drop the ME_ noise and the typedef and go for:<br>
<br>
struct re_object<br>
{<br>
   struct list entry;<br>
   REOBJECT obj;<br>
};<br>
<br>
<br>
>  typedef struct tagME_Run<br>
>  {<br>
>    ME_Style *style;<br>
> @@ -163,7 +168,7 @@ typedef struct tagME_Run<br>
>    int nFlags;<br>
>    int nAscent, nDescent; /* pixels above/below baseline */<br>
>    POINT pt; /* relative to para's position */<br>
> -  REOBJECT *ole_obj; /* FIXME: should be a union with strText (at least) */<br>
> +  ME_ReObjItem *reo_item; /* FIXME: should be a union with strText (at least) */<br>
>  <br>
>    SCRIPT_ANALYSIS script_analysis;<br>
>    int num_glyphs, max_glyphs;<br>
> @@ -436,6 +441,7 @@ typedef struct tagME_TextEditor<br>
>    BOOL bMouseCaptured;<br>
>    int wheel_remain;<br>
>    struct list style_list;<br>
> +  struct list reobj_list;<br>
>  } ME_TextEditor;<br>
>  <br>
>  typedef struct tagME_Context<br>
> diff --git a/dlls/riched20/list.c b/dlls/riched20/list.c<br>
> index 58b64e8081..f7fb1e94b6 100644<br>
> --- a/dlls/riched20/list.c<br>
> +++ b/dlls/riched20/list.c<br>
> @@ -97,7 +97,7 @@ BOOL ME_PrevRun(ME_DisplayItem **para, ME_DisplayItem **run, BOOL all_para)<br>
>    {<br>
>      if (p->type == diParagraph) {<br>
>        if (!all_para) return FALSE;<br>
> -      if (p->member.para.prev_para-><wbr>type == diParagraph)<br>
> +      if (para && p->member.para.prev_para->type == diParagraph)<br>
<br>
Why are you sneaking this change in here?  It should be a separate patch.<br>
<br>
>          *para = p->member.para.prev_para;<br>
>      } else if (p->type == diRun) {<br>
>        *run = p;<br>
> @@ -169,7 +169,11 @@ void ME_DestroyDisplayItem(ME_<wbr>DisplayItem *item)<br>
>  <br>
>    if (item->type==diRun)<br>
>    {<br>
> -    if (item->member.run.ole_obj) ME_DeleteReObject(item-><wbr>member.run.ole_obj);<br>
> +    if (item->member.run.reo_item)<br>
> +    {<br>
> +        list_remove(&item->member.run.<wbr>reo_item->entry);<br>
> +        ME_DeleteReObject(item-><wbr>member.run.reo_item->reobj);<br>
> +    }<br>
>      heap_free( item->member.run.glyphs );<br>
>      heap_free( item->member.run.clusters );<br>
>      ME_ReleaseStyle(item->member.<wbr>run.style);<br>
> diff --git a/dlls/riched20/richole.c b/dlls/riched20/richole.c<br>
> index c19a2e7ffa..d315cb9877 100644<br>
> --- a/dlls/riched20/richole.c<br>
> +++ b/dlls/riched20/richole.c<br>
> @@ -5284,11 +5284,11 @@ void ME_GetOLEObjectSize(const ME_Context *c, ME_Run *run, SIZE *pSize)<br>
>    ENHMETAHEADER emh;<br>
>  <br>
>    assert(run->nFlags & MERF_GRAPHICS);<br>
> -  assert(run->ole_obj);<br>
> +  assert(run->reo_item);<br>
>  <br>
> -  if (run->ole_obj-><a href="http://sizel.cx" rel="noreferrer" target="_blank">sizel.cx</a> != 0 || run->ole_obj-><a href="http://sizel.cy" rel="noreferrer" target="_blank">sizel.cy</a> != 0)<br>
> +  if (run->reo_item->reobj-><a href="http://sizel.cx" rel="noreferrer" target="_blank">sizel.<wbr>cx</a> != 0 || run->reo_item->reobj-><a href="http://sizel.cy" rel="noreferrer" target="_blank">sizel.cy</a> != 0)<br>
>    {<br>
> -    convert_sizel(c, &run->ole_obj->sizel, pSize);<br>
> +    convert_sizel(c, &run->reo_item->reobj->sizel, pSize);<br>
>      if (c->editor->nZoomNumerator != 0)<br>
>      {<br>
>        pSize->cx = MulDiv(pSize->cx, c->editor->nZoomNumerator, c->editor->nZoomDenominator);<br>
> @@ -5297,13 +5297,13 @@ void ME_GetOLEObjectSize(const ME_Context *c, ME_Run *run, SIZE *pSize)<br>
>      return;<br>
>    }<br>
>  <br>
> -  if (!run->ole_obj->poleobj)<br>
> +  if (!run->reo_item->reobj-><wbr>poleobj)<br>
>    {<br>
>      pSize->cx = pSize->cy = 0;<br>
>      return;<br>
>    }<br>
>  <br>
> -  if (IOleObject_QueryInterface(<wbr>run->ole_obj->poleobj, &IID_IDataObject, (void**)&ido) != S_OK)<br>
> +  if (IOleObject_QueryInterface(<wbr>run->reo_item->reobj->poleobj, &IID_IDataObject, (void**)&ido) != S_OK)<br>
>    {<br>
>        FIXME("Query Interface IID_IDataObject failed!\n");<br>
>        pSize->cx = pSize->cy = 0;<br>
> @@ -5366,13 +5366,13 @@ void ME_DrawOLE(ME_Context *c, int x, int y, ME_Run *run, BOOL selected)<br>
>    RECT          rc;<br>
>  <br>
>    assert(run->nFlags & MERF_GRAPHICS);<br>
> -  assert(run->ole_obj);<br>
> -  if (IOleObject_QueryInterface(<wbr>run->ole_obj->poleobj, &IID_IDataObject, (void**)&ido) != S_OK)<br>
> +  assert(run->reo_item);<br>
> +  if (IOleObject_QueryInterface(<wbr>run->reo_item->reobj->poleobj, &IID_IDataObject, (void**)&ido) != S_OK)<br>
>    {<br>
>      FIXME("Couldn't get interface\n");<br>
>      return;<br>
>    }<br>
> -  has_size = run->ole_obj-><a href="http://sizel.cx" rel="noreferrer" target="_blank">sizel.cx</a> != 0 || run->ole_obj-><a href="http://sizel.cy" rel="noreferrer" target="_blank">sizel.cy</a> != 0;<br>
> +  has_size = run->reo_item->reobj-><a href="http://sizel.cx" rel="noreferrer" target="_blank">sizel.cx</a> != 0 || run->reo_item->reobj-><a href="http://sizel.cy" rel="noreferrer" target="_blank">sizel.cy</a> != 0;<br>
>    fmt.cfFormat = CF_BITMAP;<br>
>    fmt.ptd = NULL;<br>
>    fmt.dwAspect = DVASPECT_CONTENT;<br>
> @@ -5399,7 +5399,7 @@ void ME_DrawOLE(ME_Context *c, int x, int y, ME_Run *run, BOOL selected)<br>
>      old_bm = SelectObject(hMemDC, stgm.u.hBitmap);<br>
>      if (has_size)<br>
>      {<br>
> -      convert_sizel(c, &run->ole_obj->sizel, &sz);<br>
> +      convert_sizel(c, &run->reo_item->reobj->sizel, &sz);<br>
>      } else {<br>
>        <a href="http://sz.cx" rel="noreferrer" target="_blank">sz.cx</a> = dibsect.dsBm.bmWidth;<br>
>        <a href="http://sz.cy" rel="noreferrer" target="_blank">sz.cy</a> = dibsect.dsBm.bmHeight;<br>
> @@ -5419,7 +5419,7 @@ void ME_DrawOLE(ME_Context *c, int x, int y, ME_Run *run, BOOL selected)<br>
>      GetEnhMetaFileHeader(stgm.u.<wbr>hEnhMetaFile, sizeof(emh), &emh);<br>
>      if (has_size)<br>
>      {<br>
> -      convert_sizel(c, &run->ole_obj->sizel, &sz);<br>
> +      convert_sizel(c, &run->reo_item->reobj->sizel, &sz);<br>
>      } else {<br>
>        <a href="http://sz.cx" rel="noreferrer" target="_blank">sz.cx</a> = emh.rclBounds.right - emh.rclBounds.left;<br>
>        <a href="http://sz.cy" rel="noreferrer" target="_blank">sz.cy</a> = emh.rclBounds.bottom - emh.rclBounds.top;<br>
> diff --git a/dlls/riched20/run.c b/dlls/riched20/run.c<br>
> index 098c4f8b4d..1cf96b54c8 100644<br>
> --- a/dlls/riched20/run.c<br>
> +++ b/dlls/riched20/run.c<br>
> @@ -290,7 +290,7 @@ ME_DisplayItem *ME_MakeRun(ME_Style *s, int nFlags)<br>
>  {<br>
>    ME_DisplayItem *item = ME_MakeDI(diRun);<br>
>    item->member.run.style = s;<br>
> -  item->member.run.ole_obj = NULL;<br>
> +  item->member.run.reo_item = NULL;<br>
>    item->member.run.nFlags = nFlags;<br>
>    item->member.run.nCharOfs = -1;<br>
>    item->member.run.len = 0;<br>
> diff --git a/dlls/riched20/writer.c b/dlls/riched20/writer.c<br>
> index aad2e44879..7f170d20bc 100644<br>
> --- a/dlls/riched20/writer.c<br>
> +++ b/dlls/riched20/writer.c<br>
> @@ -947,7 +947,7 @@ static BOOL stream_out_graphics( ME_TextEditor *editor, ME_OutStream *stream,<br>
>      SIZE goal, pic;<br>
>      ME_Context c;<br>
>  <br>
> -    hr = IOleObject_QueryInterface( run->ole_obj->poleobj, &IID_IDataObject, (void **)&data );<br>
> +    hr = IOleObject_QueryInterface( run->reo_item->reobj->poleobj, &IID_IDataObject, (void **)&data );<br>
>      if (FAILED(hr)) return FALSE;<br>
>  <br>
>      ME_InitContext( &c, editor, ITextHost_TxGetDC( editor->texthost ) );<br>
> @@ -975,8 +975,8 @@ static BOOL stream_out_graphics( ME_TextEditor *editor, ME_OutStream *stream,<br>
>                       emf_bits->szlMillimeters.cy * <a href="http://c.dpi.cy" rel="noreferrer" target="_blank">c.dpi.cy</a> * 10 );<br>
>  <br>
>      /* convert goal size to twips */<br>
> -    <a href="http://goal.cx" rel="noreferrer" target="_blank">goal.cx</a> = MulDiv( run->ole_obj-><a href="http://sizel.cx" rel="noreferrer" target="_blank">sizel.cx</a>, 144, 254 );<br>
> -    <a href="http://goal.cy" rel="noreferrer" target="_blank">goal.cy</a> = MulDiv( run->ole_obj-><a href="http://sizel.cy" rel="noreferrer" target="_blank">sizel.cy</a>, 144, 254 );<br>
> +    <a href="http://goal.cx" rel="noreferrer" target="_blank">goal.cx</a> = MulDiv( run->reo_item->reobj-><a href="http://sizel.cx" rel="noreferrer" target="_blank">sizel.cx</a><wbr>, 144, 254 );<br>
> +    <a href="http://goal.cy" rel="noreferrer" target="_blank">goal.cy</a> = MulDiv( run->reo_item->reobj-><a href="http://sizel.cy" rel="noreferrer" target="_blank">sizel.cy</a><wbr>, 144, 254 );<br>
>  <br>
>      if (!ME_StreamOutPrint( stream, "{\\*\\shppict{\\pict\\<wbr>emfblip\\picw%d\\pich%d\\<wbr>picwgoal%d\\pichgoal%d\n",<br>
>                              <a href="http://pic.cx" rel="noreferrer" target="_blank">pic.cx</a>, <a href="http://pic.cy" rel="noreferrer" target="_blank">pic.cy</a>, <a href="http://goal.cx" rel="noreferrer" target="_blank">goal.cx</a>, <a href="http://goal.cy" rel="noreferrer" target="_blank">goal.cy</a> ))<br>
> <br>
<br>
> <br>
<br>
<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Jactry Zeng</div><div><br></div></div></div>
</div>