Loading ...
Sorry, an error occurred while loading the content.
 

Re: [PATCH] IM support with ATSUI rendering

Expand Messages
  • björn
    ... The way it is done is by using int utf_char2cells(int c); which is defined in src/mbyte.c. Of course, being part of Vim you cannot call this function from
    Message 1 of 16 , Sep 2, 2008
      2008/9/2 Kaoru YOSHIDA <to.kaoru@...>:
      >
      > I tried to use drawString(). The problem when I used drawString() here
      > is that I couldn't find a way to set "flags". You know the "flags" must
      > change depending on whether "string" consists of single or multiple
      > byte characters. I have no idea how to get such information...
      > Is there any way to do that?

      The way it is done is by using

      int utf_char2cells(int c);

      which is defined in src/mbyte.c. Of course, being part of Vim you
      cannot call this function from MacVim (it is not linked to the MacVim
      executable). You could cut and paste parts of that function (some
      bits of it you would not need) I guess but that's not very elegant.
      Unfortunately, I don't know any other way to do this.

      > If I use drawAtPoint(), I don't need to care about such issue. So I
      > would prefer to using drawAtPoint() to draw characters with Input
      > Manager, unless there's an issue to use drawAtPoint().

      Well, if it works ok then the difficulties in using drawString: may
      not be worth the effort. I don't mind sticking with that and seeing
      how it goes for now.

      Björn

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_mac" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Kaoru YOSHIDA
      Hi Jiang and Björn, ... This is a result of my trial-and-error hacking. The motivation to do so is to draw IM characters properly when the length of IM
      Message 2 of 16 , Sep 2, 2008
        Hi Jiang and Björn,

        On 2008/09/03, at 0:41, Jjgod Jiang wrote:

        > Nice, that's what I've been thinking. As I skimmed through the patch
        > (btw, you seemed to attached a patch in quoted-printable encoding
        > instead of plain text), the following logic seemed unclear to me:
        >
        > + if ([text isKindOfClass:[NSAttributedString class]]) {
        > + [self setMarkedTextAttributes:
        > + [NSDictionary dictionaryWithObjectsAndKeys:
        > + [ts fontWide], NSFontAttributeName,
        > ...
        > + nil]];
        > + } else {
        > + [self setMarkedTextAttributes:
        > + [NSDictionary dictionaryWithObjectsAndKeys:
        > + [ts font], NSFontAttributeName,
        > ...
        > + nil]];
        > }
        >
        > Could you explain why would you choose 'fontWide' for marked
        > text in NSAttributedString and 'font' for NSString? Because
        > AFAIK, a marked text can contain both CJK characters *and*
        > Latin characters, say, I'm composing a Chinese word "如果",
        > then in certain state the marked text will be "如guo". In
        > that case, choosing 'fontWide' will not be appropriate since
        > normally we don't want to use 'fontWide' for Latin characters
        > "guo".

        This is a result of my trial-and-error hacking. The motivation
        to do so is to draw IM characters properly when the length of IM
        characters is longer than single line. Say if your window has 80
        single byte character width for each line, and you start typing
        IM characters at 77th column. In such a case, the third character
        should appear at the next line as long as you type only two-byte
        characters, right? To achieve this, I think an IM character should
        have the same character width of a character which is drawn by
        'native' MacVim. Based on this motivation, I introduced 'fontWide'.
        So, this is somehow related to what you pointed out past.

        My open question is why native and IM characters have different
        character width. If both has the same width, we don't need to
        be bothered this...

        >> One big remaining implementation is -firstRectForCharacterRange in
        >> MMAtsuiTextView. Since I couldn't figure out a way to implement this,
        >> I really appreciate if anyone can do it.
        >
        > I think you can use preEdit{Row,Column} to calculate the rect.
        > In vim-cocoa, I used the following code:

        Great!
        I implemented new code based on yours. Since I would like to show
        a pop-up window more precious position, I added a little hack to
        check which IM characters are selected.

        Thanks,

        -- kaoru


        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_mac" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • björn
        Hi Kaoru,Thanks for the patch(es). I ve looked at the last one and it is starting to look like it could be merged soon. I did some cleanup to your patch
        Message 3 of 16 , Sep 3, 2008
          Hi Kaoru,

          Thanks for the patch(es). I've looked at the last one and it is
          starting to look like it could be merged soon. I did some cleanup to
          your patch and am attaching it to this post (my cleanup is in a second
          commit on top of yours so that you may easily see what I have
          changed).

          2008/9/3 Kaoru YOSHIDA <to.kaoru@...>:
          >>
          >> Could you explain why would you choose 'fontWide' for marked
          >> text in NSAttributedString and 'font' for NSString? Because
          >> AFAIK, a marked text can contain both CJK characters *and*
          >> Latin characters, say, I'm composing a Chinese word "如果",
          >> then in certain state the marked text will be "如guo". In
          >> that case, choosing 'fontWide' will not be appropriate since
          >> normally we don't want to use 'fontWide' for Latin characters
          >> "guo".
          >
          > This is a result of my trial-and-error hacking. The motivation
          > to do so is to draw IM characters properly when the length of IM
          > characters is longer than single line. Say if your window has 80
          > single byte character width for each line, and you start typing
          > IM characters at 77th column. In such a case, the third character
          > should appear at the next line as long as you type only two-byte
          > characters, right? To achieve this, I think an IM character should
          > have the same character width of a character which is drawn by
          > 'native' MacVim. Based on this motivation, I introduced 'fontWide'.
          > So, this is somehow related to what you pointed out past.
          >
          > My open question is why native and IM characters have different
          > character width. If both has the same width, we don't need to
          > be bothered this...

          I think there is some kind of misunderstanding here (perhaps on my
          part) so I'll try to clarify.

          There are two different types of characters in Vim: "normal" and
          "wide". The former take up one column whereas the latter take up two.
          This distinction is in Vim and has nothing to do with IM. In your IM
          code you have the check Jiang pointed out which determines if a whole
          string of characters (the marked text) is wide or not by checking if
          the string is of type NSAttributedString or NSString. This test, as
          Jiang points out, is not correct when there are both wide and normal
          characters in the marked text. The only way I can think of how this
          (and other related problems in the IM code) could be fixed is to bring
          over the utf_char2cells() function from Vim and use that to determine
          which characters are wide or not.

          >>> One big remaining implementation is -firstRectForCharacterRange in
          >>> MMAtsuiTextView. Since I couldn't figure out a way to implement this,
          >>> I really appreciate if anyone can do it.
          >>
          >> I think you can use preEdit{Row,Column} to calculate the rect.
          >> In vim-cocoa, I used the following code:
          >
          > Great!
          > I implemented new code based on yours. Since I would like to show
          > a pop-up window more precious position, I added a little hack to
          > check which IM characters are selected.

          That looks better, but perhaps you can "merge" the MMTextView and
          MMAtsuiTextView code and put it in the helper class so that both ATSUI
          and the default renderer may benefit from the better placement of the
          IM popup window? (You'll have to wrap the
          boundingRectForCharacterAtRow:column: somehow to do this. You may
          want to make the convertRow:column:toPoint: method public and
          implement a similar one in ATSUI and then call this from the helper
          and get rid of the boundingRectFor... method.) In my attached patch I
          did something similar for setMarkedText: but I ran out of time for now
          so maybe you can complete that part?

          I've noticed that there are still some IM aspects that are missing in
          ATSUI; e.g. there is no insertion point when editing marked text. If
          you find the time it would be great if you could make the two
          renderers have the same features. :-)

          Thanks for your work,
          Björn

          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_mac" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • Kaoru YOSHIDA
          Hi Björn, Sorry for late reply. I was kinda busy these days... ... I merged firstRectForCharacterRange: method into MMTextViewHelper class. In
          Message 4 of 16 , Sep 8, 2008
            Hi Björn,

            Sorry for late reply. I was kinda busy these days...

            On 2008/09/04, at 4:26, björn wrote:

            >

            > That looks better, but perhaps you can "merge" the MMTextView and
            > MMAtsuiTextView code and put it in the helper class so that both ATSUI
            > and the default renderer may benefit from the better placement of the
            > IM popup window? (You'll have to wrap the
            > boundingRectForCharacterAtRow:column: somehow to do this. You may
            > want to make the convertRow:column:toPoint: method public and
            > implement a similar one in ATSUI and then call this from the helper
            > and get rid of the boundingRectFor... method.) In my attached patch I
            > did something similar for setMarkedText: but I ran out of time for now
            > so maybe you can complete that part?

            I merged firstRectForCharacterRange: method into MMTextViewHelper
            class. In MMTextViewHelper, I wrote the following code to check
            whether textView is MMTextView or MMAtsuiTextView. Since coordinate
            axes of MMTextView and MMAtsuiTextView is different, I need to check
            it.

            + if (![textView isKindOfClass:[MMTextView class]]) {
            + row = [textView maxRows] - row;
            + on = -1;
            + }

            This may not be a good idea to do this in MMTextViewHelper. If not
            appropriate, we should define a new function in MMAtsuiTextView
            and call it from MMTextViewHelper. Since no appropriate function
            name comes up to my mind, I didn't implement that way.
            The patch file is 0003-XXXX.

            > I've noticed that there are still some IM aspects that are missing in
            > ATSUI; e.g. there is no insertion point when editing marked text. If
            > you find the time it would be great if you could make the two
            > renderers have the same features. :-)

            I implemented this part in 0004-XXXX.
            While I use drawInsertionPointAtRow: method to draw the insertion
            point, the arguments are hard coded. If there's a smarter way,
            could you let me know?

            The patch 0005-XXX is to fix a drawing issue of Japanese/Chinese
            characters with ATSUI rendering. So, the problem is when shifting
            down those characters one or more lines, small black dots show up.
            I don't know this is a proper way to fix it, but this fixes the
            issue at least in my environment.

            0001-XXX and 0002-XXX are patches you posted.

            Thanks,

            -- kaoru



            --~--~---------~--~----~------------~-------~--~----~
            You received this message from the "vim_mac" maillist.
            For more information, visit http://www.vim.org/maillist.php
            -~----------~----~----~----~------~----~------~--~---
          • Kaoru YOSHIDA
            Hi Björn, Sorry for contiguous posts, but my last patches aren t compilable. The attached one should fix the issue. Thanks, -- kaoru ...
            Message 5 of 16 , Sep 9, 2008
              Hi Björn,

              Sorry for contiguous posts, but my last patches aren't
              compilable. The attached one should fix the issue.

              Thanks,

              -- kaoru

              On 2008/09/09, at 11:45, Kaoru YOSHIDA wrote:

              > Hi Björn,
              >
              > Sorry for late reply. I was kinda busy these days...
              >
              > On 2008/09/04, at 4:26, björn wrote:
              >
              >>
              >
              >> That looks better, but perhaps you can "merge" the MMTextView and
              >> MMAtsuiTextView code and put it in the helper class so that both
              >> ATSUI
              >> and the default renderer may benefit from the better placement of the
              >> IM popup window? (You'll have to wrap the
              >> boundingRectForCharacterAtRow:column: somehow to do this. You may
              >> want to make the convertRow:column:toPoint: method public and
              >> implement a similar one in ATSUI and then call this from the helper
              >> and get rid of the boundingRectFor... method.) In my attached
              >> patch I
              >> did something similar for setMarkedText: but I ran out of time for
              >> now
              >> so maybe you can complete that part?
              >
              > I merged firstRectForCharacterRange: method into MMTextViewHelper
              > class. In MMTextViewHelper, I wrote the following code to check
              > whether textView is MMTextView or MMAtsuiTextView. Since coordinate
              > axes of MMTextView and MMAtsuiTextView is different, I need to check
              > it.
              >
              > + if (![textView isKindOfClass:[MMTextView class]]) {
              > + row = [textView maxRows] - row;
              > + on = -1;
              > + }
              >
              > This may not be a good idea to do this in MMTextViewHelper. If not
              > appropriate, we should define a new function in MMAtsuiTextView
              > and call it from MMTextViewHelper. Since no appropriate function
              > name comes up to my mind, I didn't implement that way.
              > The patch file is 0003-XXXX.
              >
              >> I've noticed that there are still some IM aspects that are missing in
              >> ATSUI; e.g. there is no insertion point when editing marked text. If
              >> you find the time it would be great if you could make the two
              >> renderers have the same features. :-)
              >
              > I implemented this part in 0004-XXXX.
              > While I use drawInsertionPointAtRow: method to draw the insertion
              > point, the arguments are hard coded. If there's a smarter way,
              > could you let me know?
              >
              > The patch 0005-XXX is to fix a drawing issue of Japanese/Chinese
              > characters with ATSUI rendering. So, the problem is when shifting
              > down those characters one or more lines, small black dots show up.
              > I don't know this is a proper way to fix it, but this fixes the
              > issue at least in my environment.
              >
              > 0001-XXX and 0002-XXX are patches you posted.
              >
              > Thanks,
              >
              > -- kaoru
              >
              >
              > <IM080909.tar.bz2>


              --~--~---------~--~----~------------~-------~--~----~
              You received this message from the "vim_mac" maillist.
              For more information, visit http://www.vim.org/maillist.php
              -~----------~----~----~----~------~----~------~--~---
            • björn
              Hi Kaoru, ... Ok, that looks a bit weird so I fixed it. Instead of manually converting from (rows,columns) to a pixel value I now supply the methods: -
              Message 6 of 16 , Sep 9, 2008
                Hi Kaoru,

                2008/9/9 Kaoru YOSHIDA <to.kaoru@...>:
                >
                > I merged firstRectForCharacterRange: method into MMTextViewHelper
                > class. In MMTextViewHelper, I wrote the following code to check
                > whether textView is MMTextView or MMAtsuiTextView. Since coordinate
                > axes of MMTextView and MMAtsuiTextView is different, I need to check
                > it.
                >
                > + if (![textView isKindOfClass:[MMTextView class]]) {
                > + row = [textView maxRows] - row;
                > + on = -1;
                > + }
                >
                > This may not be a good idea to do this in MMTextViewHelper. If not
                > appropriate, we should define a new function in MMAtsuiTextView
                > and call it from MMTextViewHelper. Since no appropriate function
                > name comes up to my mind, I didn't implement that way.
                > The patch file is 0003-XXXX.

                Ok, that looks a bit weird so I fixed it. Instead of "manually"
                converting from (rows,columns) to a pixel value I now supply the
                methods:

                - (NSPoint)pointForRow:(int)row column:(int)col;
                - (NSRect)rectForRow:(int)row column:(int)col numRows:(int)nr
                numColumns:(int)nc;

                I amended your patch number 0003 using those methods instead so that
                the helper does not need to check if the text view is ATSUI or Cocoa
                (attached).

                While I was at it I fixed a minor detail in patch number 0002 and the
                amended patch is also in the attached archive.

                >
                >> I've noticed that there are still some IM aspects that are missing in
                >> ATSUI; e.g. there is no insertion point when editing marked text. If
                >> you find the time it would be great if you could make the two
                >> renderers have the same features. :-)
                >
                > I implemented this part in 0004-XXXX.
                > While I use drawInsertionPointAtRow: method to draw the insertion
                > point, the arguments are hard coded. If there's a smarter way,
                > could you let me know?

                I've run out of time now, but I'll check it tomorrow probably. The
                color for the cursor could be taken from defaultForegroundColor I
                suppose, but as for the shape and percentage I guess you just have to
                hard-code it.

                > The patch 0005-XXX is to fix a drawing issue of Japanese/Chinese
                > characters with ATSUI rendering. So, the problem is when shifting
                > down those characters one or more lines, small black dots show up.
                > I don't know this is a proper way to fix it, but this fixes the
                > issue at least in my environment.

                That looks a bit weird, but I haven't had a time to check. What
                happens if you change the text inset to something big like 10 pixels?
                Isn't the text going to be drawn in the wrong place then? You can try
                by typing the following line in Terminal:

                defaults write org.vim.MacVim MMTextInsetTop 10

                and restart. When you're done, enter the following in Terminal:

                defaults delete org.vim.MacVim MMTextInsetTop

                The patches I've attached replaces the corresponding ones in your
                archive...so you'll still need to apply your patch 0001 before the two
                I attach.

                I'll get back to you when I've had a chance to look at patch 0004 and 0005.

                Thanks,
                Björn

                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_mac" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              • björn
                ... I have updated this patch a bit so that the color is now the proper cursor color (and not the foreground color as I suggested above). It is attached as an
                Message 7 of 16 , Sep 10, 2008
                  2008/9/9 björn <bjorn.winckler@...>:
                  >>
                  >>> I've noticed that there are still some IM aspects that are missing in
                  >>> ATSUI; e.g. there is no insertion point when editing marked text. If
                  >>> you find the time it would be great if you could make the two
                  >>> renderers have the same features. :-)
                  >>
                  >> I implemented this part in 0004-XXXX.
                  >> While I use drawInsertionPointAtRow: method to draw the insertion
                  >> point, the arguments are hard coded. If there's a smarter way,
                  >> could you let me know?
                  >
                  > I've run out of time now, but I'll check it tomorrow probably. The
                  > color for the cursor could be taken from defaultForegroundColor I
                  > suppose, but as for the shape and percentage I guess you just have to
                  > hard-code it.

                  I have updated this patch a bit so that the color is now the proper
                  cursor color (and not the foreground color as I suggested above). It
                  is attached as an archive.

                  >> The patch 0005-XXX is to fix a drawing issue of Japanese/Chinese
                  >> characters with ATSUI rendering. So, the problem is when shifting
                  >> down those characters one or more lines, small black dots show up.
                  >> I don't know this is a proper way to fix it, but this fixes the
                  >> issue at least in my environment.
                  >
                  > That looks a bit weird, but I haven't had a time to check. What
                  > happens if you change the text inset to something big like 10 pixels?
                  > Isn't the text going to be drawn in the wrong place then?

                  This patch is indeed not working properly. You can't add the text
                  inset to the drawing position in that way. What is the problem that
                  you were trying to fix exactly? (A screenshot might help.)

                  There are still some problems that I have noticed.

                  1. The IM "popup" window is often too far to the right: see the
                  attached screenshots

                  2. The spacing of roman letters is sometimes wide when it should be
                  normal as is also exhibited in the screenshot. This is the issue
                  Jiang brought up earlier and which I've commented on twice before.

                  I realize these issues are a bit hard to deal with because at the
                  moment there simply isn't enough information to "get it right". If
                  you find the time, maybe have a look at duplicating some of the code
                  utf_char2cells() inside mbyte.c. Using such a function you would be
                  able to decide whether to use the wide or normal font and that would
                  potentially solve both of the above issues, but it would also
                  complicate your existing code quite a bit. Since I don't use IM I
                  can't really judge whether it is "worth it" to implement this just see
                  this as an idea and decide for yourself if it makes sense.

                  Finally, please look over the modified patched I've posted here and in
                  my previous post (numbers 2, 3 and 4) and let me know if they look ok
                  to you and I can start merging.

                  Thanks,
                  Björn

                  --~--~---------~--~----~------------~-------~--~----~
                  You received this message from the "vim_mac" maillist.
                  For more information, visit http://www.vim.org/maillist.php
                  -~----------~----~----~----~------~----~------~--~---
                • björn
                  ... Sorry, I forgot the screenshots. I m attaching them to this post. Björn --~--~---------~--~----~------------~-------~--~----~ You received this message
                  Message 8 of 16 , Sep 10, 2008
                    > There are still some problems that I have noticed.
                    >
                    > 1. The IM "popup" window is often too far to the right: see the
                    > attached screenshots
                    >
                    > 2. The spacing of roman letters is sometimes wide when it should be
                    > normal as is also exhibited in the screenshot. This is the issue
                    > Jiang brought up earlier and which I've commented on twice before.

                    Sorry, I forgot the screenshots. I'm attaching them to this post.

                    Björn

                    --~--~---------~--~----~------------~-------~--~----~
                    You received this message from the "vim_mac" maillist.
                    For more information, visit http://www.vim.org/maillist.php
                    -~----------~----~----~----~------~----~------~--~---
                  Your message has been successfully submitted and would be delivered to recipients shortly.