News

Code Review ของ SkThaiPreprocessor ผ่าน Twitter

virasak: ผมทำ fixThaiVowel version ใหม่เสร็จแล้ว เขียน style คุณ @chanwit แต่จะไม่มีเก็บสถานะตัวอักษรก่อนหน้านอก function นี้
chanwit: ไม่ hold state ไว้แล้วทำไงครับ ? ส่งมาเลยครับ
virasak: เพราะอันเก่าบางจุดผมยังงงอยู่และก็สงสัยเรื่อง ThaiPreprocessor มัน instance เดียวแต่เก็บสภานะตัวอักษรของ text ที่กำลังทำงานปัจจุบัน
chanwit: ของ ThaiPreprocessor เป็นเพราะ stream ของ data มันมาจากด้านนอกครับ จาก SkPaint ไม่ได้เอา text ทั้งสายมา process เอง
virasak: จะส่งให้ทาง mail นะครับ
chanwit: ผมว่าจะยุให้หัด git 🙂
virasak: ครับผมก็เลยงงหลายจุด โดยเฉพาะ if ที่ check ว่าเป็นตัวอักษรที่โดน shift แล้ว หรือไม่ เพราะว่า text ที่เข้ามาไม่น่าจะเปลี่ยนตามresult
virasak: ตอนนี้ติดตรงไม่มี harddisk ครับ ที่ไม่พอก็เลยเขียนกับ Java ไปก่อน
chanwit: ใช่ครับ เพราะ result มันถูกส่งออกไปเข้า glyph cache แล้วเอาไปวาดบนหน้าจอเลย
chanwit: มีการสร้าง cache obj ให้ text 1 สายครับ เลยรับประกันได้ว่าถ้า preprocessor เป็น instance var ของ cache แล้ว state จะไม่ share กัน
virasak: แต่ check กับ ch pch ซึ่งได้มาจาก text ไม่ได้จาก result ในการแปลง ดังนั้น ch pch น่าจะมีค่าได้แค่ตัวอักษรที่ยังไม่ process
virasak: } else if (isLeftShiftUpperLevel1(temp_pch)) {
virasak: อันนี้ check ว่า temp_pch เป็นตัวที่ shift ไปแล้วหรือไม่ แต่ว่า temp_pch ได้จาก pch ที่ได้จาก ch และ ch ไม่เคยรับจาก result
chanwit: ผมใช้เทคนิค windowing ค่อย ๆ เลื่อน window โดยใช้ pch0, pch, ch, ch1 จับ current state ของ text หน่ะครับ
chanwit: แล้วทยอยเลื่อนค่า pch0 = pch, pch = ch ไปเรื่อย ๆ
virasak: อ้อ ครับ ถ้างั้นประเด็น share คงไม่มีปัญหาแล้ว
chanwit: จุดที่บอก มันทำให้เกิด bug เหรอครับ ?
virasak: current state ตัวนี้มาจาก text ใช่มั้ยครับ ถ้า result ไมได้กลับมาเปลี่ยน ค่าใน text
virasak: แสดงว่า current state ก็จะไม่มีทาง เป็นค่า ที่ถูก shift ได้ ดังนั้นการ check ว่า shift มั้ย บน state เหล่านี้ จะไม่มีทางเป็นจริง
chanwit: ข้อจำกัดคือ 1. ต้อง assume ว่า text เป็น read-only, 2. อ่าน ch แบบไปข้างหน้าได้อย่างเดียวครับ
virasak: ลืมใส่ reply ทุกที
virasak: ลืมอีกแล้ว
chanwit: โอเค get แล้วครับ น่าจะเป็น bug แน่เพราะ ไม่ได้เก็บ state ย้อนหลังที่ leftshift ไว้
virasak: ถ้า text เป็น read only แสดงว่า ch pch temp_pch จะต้อง เป็นค่าที่ยังไม่ได้ถูกแปลงใช่มั้ยครับ เพราะได้จาก text เท่านั้น
chanwit: ใช่ครับ
virasak: ตัวที่ผมส่งให้ หลักๆน่าจะแก้สองจุดที่พูดไป เรื่อง ไม่ share state ก็ทำให้เราเอา pch ออกมาจาก fixThaiVowel ได้
virasak: ตัวนี้ก็จะแก้ที่จุดเดียวคือ check กับข้อมูลที่ยังไม่ได้แปลเท่านั้น และผมพยายามเขียนให้มันแก้ง่ายอาจจะยาวหน่อย ถ้า work แล้วค่อยปรับ
chanwit: ดูโค้ดแล้ว เอาไปใช้ไม่ได้ครับ text ตัวนั้นใน android มันไม่มี index
chanwit: ถ้าจะอ่านค่ากลับแบบโค้ดชุดใหม่ต้อง buffer data ไว้ = เสีย memory 2 เท่า แต่ถ้าไม่ทำแบบนั้นต้อง parse UTF ย้อนหลัง = เลื่อน pointer
virasak: จะมีก็แค่ตัวที่รับ i – 1
virasak: ไม่รู้ว่าเขามี prev สองครั้งได้หรือปล่าว
virasak: ที่ต้องมี prev ย้อนไปสองตัว เพราะเรื่อง share stateถ้าไม่มีปัญหา แสดงว่าเรามาเก็บ state ย้อนหลังไว้ข้างนอกเหมือนเดิม ก็น่าจะได้
chanwit: อ๋อ โอเค ครับ
virasak: prevChar(content, i) เทียบเท่า SkUTFXX_PrevUnichar(text)
chanwit: ผมไม่เห็น isLeftShiftUpperLevel1 แล้ว โค้ดนี้แก้ไปแล้วเหรอครับ
virasak: เท่าที่ดู code prevChar(content, i-1) == pch0 ที่เก็บ state ไว้ด้านนอก
chanwit: prevChar(content, i-1) == pch0 ใช่ครับ
virasak: prevChar(content,i) == pch ที่เก็บ state ไว้ด้านนอก
virasak: ผมว่า ผมเขียนเอา state มาเก็บนอก fixThaiVowel ก่อนดีกว่า ใน fixThaiVowel จะได้ไม่มีการเรียก prev/next
chanwit: พอเข้าใจแล้วครับ
virasak: มันไม่ได้ใช้ก็เลยลบออกไปครับ เพราะเราจะ check เฉพาะค่าที่ไม่แปลง แต่ถ้าเราเปลี่ยนมาเก็บ prev_result ด้วย ก็น่าจะเอากลับมาใช้ได้
virasak: ผมขอปรับก่อนดีกว่า
chanwit: อาจจะต้องทำแบบนั้นครับ มี case นี้ case เดียวเหรอครับ ?
virasak: } else if (isLowerLevel(ch) && !(isCutTail(pch))) { ด้วยครับ
virasak: SkUTF8_NextUnichar(text) หมายถึงตำแหน่งปัจจุบันใช่มั้ยครับ
chanwit: ใช่ครับ ปัจจุบัน แล้วเลื่อน 1 ครับ
virasak: แสดงว่า nextChar ของผม ไม่เทียบเท่า SkUTFX_NextUnichar
virasak: แสดงว่า next ครั้งที่สองเราก็จะได้ตัวถัดไปเหรอครับ
chanwit: ใช่ครับ
virasak: แล้ว pointer มันเป็น local เลือนไปไม่มีผลกับการเลื่อนของ loop ด้านนอกที่เรียกเข้ามา
virasak: แสดงว่าเราอาจจะไม่จำเป็นต้องเก็บตัวอื่นนอกจาก pch เพราะตัวอื่นใช้น่อย มาเล่นกับ pointer จะดีกว่าสำหรับตัวอื่นที่ไม่ค่อยได้ใช้
chanwit: อืม pointer มันเลื่อนแบบ global ครับ ptr ของ text ขยับจริงครับ เป็น pass-by-ref
virasak: ขอบคุณครับ เข้าใจ api มาขึ้น จะได้เลียนแบบด้วย java ใช้ iterator ไปกลับ แทน api ชุดนี่น่าจะได้
chanwit: ใช่ครับ มันคือ iterator pattern 🙂
chanwit: โอ้ เล่นกับ pointer ไม่ได้ครับ มัน expensive เพราะ step ในการเลื่อนเป็นการ decode UTF ด้วยครับ ไม่ได้เลื่อนอย่างเดียว
virasak: แสดงว่า เราก็ต้อง copy pointer เอา แล้วใช้ local แทน ต้องกลับไปดู c++ เรื่องการ pass pointer แล้ว ลืมๆไปหมดแล้ว
virasak: อ้อใช่ครับ ลืมนึกถึงจุดนี้
virasak: ทำยังไงเราถึงจะรู้ตัวถัดไป เพื่อที่จะเปลี่ยนตัวปัจจุบัน อย่างตัดเชิง ต้องรู้ว่าตัวถัดไปเป็นสระล่าง
chanwit: เป็น if ยาว ๆ ก่อน fixVowel ที่ผม next แล้ว prev หน่ะครับ ที่ตัดสินใจใช้แบบนั้นเพราะตัดเชิงเป็น rare case
virasak: อ้อ ก็ใช้อยู่ตรง if(isDownTail(ch) || isUpperLevel2(ch)) { ch1 = SkUTF8_NextUnichar(text); SkUTF8_PrevUnichar(tex t); } มี guard
virasak: next สองครั้งแล้วย้อนกลับ
chanwit: ตรงนี้เป็น bad code ครับ 555 ถ้าเอาโค้ดของ next มาใช้แบบไม่ปรับ pointer น่าจะดีกว่า แต่ case มันน้อย ผมเลยปะไปก่อน
virasak: น่าจะได้ครบแล้ว ต้องลองแปลง แต่ขอทำงานก่อนดีกว่า ค่อยมาต่อกลางคืน
virasak: ผมไม่มี api เลยเดาเฉพาะที่เห็น เดี๋ยวคงหาทางเอา source บางส่วนมาดู จะได้เลือก function ที่เหมาะๆมาใช้
virasak: { char** t = text; ch1 = SkUTF8_NextUnichar(t);} น่าจะ work ครับ ให้มันเลื่อน t แทน
virasak: function(char ** text) น่าจะได้ copy ใหม่ text ที่ส่งเข้ามานะครับ ดังนั้นการเลื่อน text ใน local ไม่น่าจะมีผลกลับ text ต้นฉบับ
virasak: @neonian ผมไม่ได้เขียน cpp มาน่าจะ 7 ปีแล้วครับ อาจจะหลงลืมๆ คงต้องกลับไปทบทวนก่อน
chanwit: คิดว่ามีเพราะมันเป็น double pointer นะครับ ไม่แน่ใจ ผมไม่ค่อยอยากยุ่งกับ pointer ถ้าไม่จำเป็น lol
virasak: ผมเดาว่า char** คือ char[j] โดย char คือ ตัวอักษรที่ ส่วน char[][j] ต้องมีหลายตัวเพราะ char มี 8bits แต่ต้องแทนทั้ง utf8+16

7 Comments

  1. nuuneoi Post on November 14, 2008 at 8:55 pm

    #45

    มีประโยชน์มากครับ ^^

    ถ้าโค้ดนี้ Submit ทั่วโลกคงได้ใช้โค้ด Pattern เดียวกันแน่ๆ

  2. virasak Post on November 16, 2008 at 6:11 am

    #59

    แก้ไขที่เดา char** text ผิด พอดีได้ดูที่ code แล้ว มันคือ pointer ไปที่ string ที่ encode utf6/16 เวลา next/prev มันจะไม่ได้แก้ text pointer แต่ไปเลื่อน string ที่ text pointer ชี้ ให้ชี้ไปที่ตัวอักษรต่อไป (กระโดดไปหลาย byte ขึ้นกับ encoding) ดังนั้นการ assign text ใส่ pointer ตัวอื่น แล้ว prev/next จะไม่ work

  3. virasak Post on November 16, 2008 at 6:16 am

    #60

    แนวทางการทำงานจะทำอะไรต่อครับ ผมว่าถ้ารอเขาไม่รู้จะได้ใช้ตอนไหน แต่ Mike Reed นี่เป็น เจ้าของ บริษัท Skia ก่อนที่ Google จะซื้อไป เขามา comment น่าจะมีอะไรคืบหน้าเหมือนกัน

    • nuuneoi Post on November 16, 2008 at 6:49 am

      #61

      ตอนนี้เนยเจอว่าตัว Source Code กับตัว Emu และ Release (G1) มีบางส่วนไม่เหมือนกัน ทำให้ไม่สามารถเอาระบบบางส่วนที่แก้แล้วลงเครื่องจริงได้ เนยคงจะแก้ตรงนี้แล้วอัพ Patch ไปให้เค้า

      แล้วก็คงทำส่วนอินพุตต่อ ถ้าสามารถทำจนเอาลงเครื่องจริงได้จะอัพ Patch ไปให้เค้าด่าอีกที

    • virasak Post on November 16, 2008 at 7:13 am

      #62

      ผมว่าเราน่าจะทำต่อไป เพราะตัวที่ทำก็ยังมี bug อยู่ แต่จะส่ง patch ให้เข้ามั้ย อาจจะรอให้นิ่งก่อนก็ได้ ในระหว่างนี้น่าจะมีวิธี share code กันในกลุ่ม อย่าง skia เราเอาขึ้นที่ github ได้มั้ยครับ แล้ว แก้จากตรงนั้น ไม่ต้องเอาขึ้นทั้งหมด project ไหนจะแก้ก็เอาขึ้นที่นั่น

      ถ้าทำแบบนี้ระหว่างที่พัฒนากันอยู่ ผู้ใช้จะได้ใช้จริงทันที ไม่ต้องรอทาง Google ถึงจะไม่ดีมากก็น่าจะพอทำงานได้

    • chanwit

      chanwit Post on November 16, 2008 at 10:57 am

      #70

      ผมตามต่อจากที่คุณ @virasak tweet แล้ว ก็เพิ่งเข้าใจว่า libsgl เป็น software graphic library ของ Skia จริง และดูเหมือ Mike จะเป็นคนรับผิดชอบหลักในเรื่องการแสดงผล

      เรื่อง input คงต้องยกให้เนยทำต่อเพราะทำไปเยอะแล้ว
      เรื่องแก้สระลอย ผมมองว่าน่าจะทำต่อในลักษณะว่า class hierarchy โดยให้มี SkUnicodePreprocessor เป็น super class แล้วมี class ลูก ตัวอย่าง 2 ตัว SkEnglishPreprocessor กับ SkThaiPreprocessor โดยที่ class แรกคืนค่าเลยตรง ๆ ในขณะที่ class ที่สองก็คือที่เขียนไว้แล้ว

      การเลือก preprocessor น่าจะทำด้วน factory method ได้แบบเดียวกับการใช้ locale สำหรับตัดคำใน webkit คือใช้ string เช่น
      en_US เลือก SkEnglishPreprocessor
      th_TH เลือก SkThaiPreprocessor
      เป็นต้นครับ

      เป็นแนวทางแล้วเดี๋ยวคุยกันต่อครับ

  4. justinguo Post on August 11, 2011 at 4:09 pm

    #217205

    clip on hair extensions are becoming increasingly popular throughout the world, and are one of the main fashion and beauty accessories available today.
    Whether you need to inject some extra volume to your thin, lifeles hair or you want to add length to your hairstyle, Miami hair extensions can be the perfect answer.

Leave a Reply

To Top