ノート線描画、指定桁縦線描画、折り返し桁縦線描画、を行毎ではなく一括で行うように変更 #1065
Conversation
…なく引数で渡すように変更 ノート線描画、指定桁縦線描画、折り返し桁縦線描画 は CEditView::DrawLayoutLine 内で行毎に行うのではなく、CEditView::OnPaint2 において一括で行うように変更
|
✅ Build sakura 1.0.2290 completed (commit 07981aab17 by @beru) |
There was a problem hiding this comment.
基本GOで 😃
変更内容が理解できてるかどうかに自信がないので、コメント付けて変更内容を分析してみます。
目的の分析
描画処理の負荷軽減＆高速化が目的で以下の変更を行いました。
「以下の変更」が文章になってるのを「◎◎の△△」に書き替えてみます。
- CGraphics生成のタイミングを変更
- 線描画のタイミングを変更
細かい内容は個別に分析するとして、タイミングを変えた、と言ってるように見えました。
CGraphics生成のタイミング
•CEditView::DrawLogicLine のローカル変数で SColorStrategyInfo 型のインスタンスを用意するのではなく引数で渡すように変更
◦CEditView::DrawLogicLine メソッドのローカル変数にしてしまうと、インスタンスの破棄が論理行毎に行われます。 SColorStrategyInfo::m_gr の型が CGraphics で解放処理が比較的重いようです。
引用文の最後部分を太字に変えています。
構造的にこんな感じの理由付けで、SColorStrategyInfoの宣言位置を変えたのかな？と思いました。
- CGraphics の デストラクタ が重い！（ ⇒呼出回数を減らしたい。 ）
- SColorStrategyInfo には CGraphics 型のメンバがある
- CGraphics の デストラクタ 呼出回数を減らすには SColorStrategyInfo の変更も必要。
- SColorStrategyInfo の生成＆破棄タイミングを変えて、破棄回数を減らしたい！
- CGraphics の デストラクタ 呼出回数を減らすには SColorStrategyInfo の変更も必要。
- SColorStrategyInfo には CGraphics 型のメンバがある
この変更内容について、特に問題はなさそうに思います。
SColorStrategyInfo::m_gr の型を CGraphics ⇒ CGraphics& としたら
あと1つデストラクタの呼出回数を減らせるような気がしますが、
SColorStrategyInfo のコンストラクタを変えにゃならんのを「面倒」と取るかどうかが焦点かな。
線描画のタイミング
•ノート線描画、指定桁縦線描画、折り返し桁縦線描画 は CEditView::DrawLayoutLine 内で行毎に行うのではなく、CEditView::OnPaint2 において一括で行うように変更
◦一括で行うようにしても問題が無いと思います。行毎に呼び出すのではなく一括で描画するようにすれば呼び出し回数が減るので処理負荷を下げる事が出来ます。
線描画は、始点と終点を定義して行う処理なので、
行ごとに描画するよりも画面全体で行うほうが効率は良いと思います。
考慮すべきポイントは2つあると思います。
- 線描画のタイミングを変える、は 描き順を変える です。
- 描き順が変わる影響として、従来描画されなかったケースで線が描画されるパターンがあり得ます。
- 具体的には、変更前は選択領域を反転描画する前に線を描いていましたが、変更後は選択領域を反転描画した後に線を描くように変わります。
- ざっと見、影響なさげなので問題ないと思います。
- 具体的には、変更前は選択領域を反転描画する前に線を描いていましたが、変更後は選択領域を反転描画した後に線を描くように変わります。
- 描き順が変わる影響として、従来描画されなかったケースで線が描画されるパターンがあり得ます。
- 一括描画が可能なのは 縦線 だけです。
- 変更対象の線種のうち、 ノート線は横線 です。
- 横線描画は行単位に行う必要があるので、説明と矛盾しています。
- あと、せっかく一括描画にしたのに、描画行数分繰り返してるような気がしました。
- これはあとでコメント入れます。
- 変更対象の線種のうち、 ノート線は横線 です。
総括
全体として、大きな問題はなさそうだと思っとります。
その懸念は自分も有りました。ただ実際に見比べてみると変化が無いように見えます。
変更前後を比較すると同じように描画されているようにみえます。 ノート線描画処理 |
どちらかというと関数名が誤っている雰囲気なんですね。 誤) CTextDrawer::DispNoteLine 指定桁縦線描画は |
| pInfo->m_pDispPos->GetDrawPos().y, | ||
| pInfo->m_pDispPos->GetDrawPos().y + nLineHeight, | ||
| CLayoutInt(0), | ||
| CLayoutInt(-1) |
| y0, | ||
| y1, | ||
| CLayoutInt(0), | ||
| CLayoutInt(-1) |
There was a problem hiding this comment.
ここの第４引数、第５引数(=最後の引数)は、描画対象の水平方向の範囲を桁単位で指定します。
もとの実装と変えてないっぽいので「修正すべき」かどうかは微妙ですが、
速度重視だと以下のようになる気がします。
GetTextDrawer().DispVerticalLines(
pInfo->m_gr,
y0,
y1,
GetTextArea().GetViewLeftCol(),
GetTextArea().GetRightCol()
);うわ、書いてみるとすっげーダサいｗ
（左端桁取得と右端桁取得のメソッド名に統一感がない）
まぁ一応、DispVerticalLines関数内部に同じ意味になるような細工が仕込まれているので、実影響はないと思うんですが、意図が分かりにくいと思うのでどっかで変えたいな、と思っています。
話がややこしくなるから、今回は放置っすかねぇ・・・。
|
レビューありがとうございました。マージします。 |
|
こっちはCloseされているから、ここに書くべきなのかは分かりませんが、
こうすると反転しないで描画した線が戻す処理の際に反転してしまいます。 |
|
#1072 で revert されたので |
ノート線描画、指定桁縦線描画、折り返し桁縦線描画、を行毎ではなく一括で行うように変更
PR の目的
描画処理の負荷軽減＆高速化が目的で以下の変更を行いました。
CEditView::DrawLogicLineのローカル変数でSColorStrategyInfo型のインスタンスを用意するのではなく引数で渡すように変更CEditView::DrawLogicLineメソッドのローカル変数にしてしまうと、インスタンスの破棄が論理行毎に行われます。SColorStrategyInfo::m_grの型がCGraphicsで解放処理が比較的重いようです。ノート線描画、指定桁縦線描画、折り返し桁縦線描画 は
CEditView::DrawLayoutLine内で行毎に行うのではなく、CEditView::OnPaint2において一括で行うように変更カテゴリ
PR の背景
Performance Profiler で確認すると描画処理の割合がかなり大きいので、少しでも減らせないかと対策を入れました。
PR のメリット
同等の処理をより少ない負担で行えるようになるので軽くなります。
PR のデメリット (トレードオフとかあれば)
不明
PR の影響範囲
描画処理
関連チケット
#998