Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

キーボードの文字の入力の表示の間隔設定をミリ秒に変換してからタイマーの間隔に使用するように修正 #523

Merged
merged 1 commit into from
Oct 7, 2018
Merged

キーボードの文字の入力の表示の間隔設定をミリ秒に変換してからタイマーの間隔に使用するように修正 #523

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions sakura_core/view/CEditView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,11 +384,12 @@ BOOL CEditView::Create(
}

/* キーボードの現在のリピート間隔を取得 */
int nKeyBoardSpeed;
SystemParametersInfo( SPI_GETKEYBOARDSPEED, 0, &nKeyBoardSpeed, 0 );

DWORD dwKeyBoardSpeed;
SystemParametersInfo( SPI_GETKEYBOARDSPEED, 0, &dwKeyBoardSpeed, 0 );
/* リピート速度の設定をミリ秒に変換 */
UINT uElapse = 400 - dwKeyBoardSpeed * (400 - 33) / 31;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

windows SDKの変数定義がおかしいのは知ってますが
変数には意味のある名前を付けるべきですよね。
https://ejje.weblio.jp/content/elapsed+time

uTimeoutとかuKeybordSpanとかが代案。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows SDKのその記述に問題があるかの判断は法曹界に委ねようかと思います。
もし記述が将来更新されたらそれに合わせる事にします。

代案のなかでは uTimeout が個人的に分かりやすいです。趣は違いますが uMilliseconds とかでも良いと思います。uInterval でも uHoge でもなんでもコンパイラがエラーを出さなければ…。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

400 >= dwKeyBoardSpeed * (400 - 33) / 31 であることは保証されるのですか?

Copy link
Contributor Author

@beru beru Oct 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SystemParametersInfo の呼び出しが失敗しなければ保証されます。

SystemParametersInfo( SPI_GETKEYBOARDSPEED, 0, &dwKeyBoardSpeed, 0 );

呼び出し後の dwKeyBoardSpeed の値の範囲は 0 ～ 31 です。

https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-systemparametersinfow
に書かれている記述を引用します。

Retrieves the keyboard repeat-speed setting, which is a value in the range from 0 (approximately 2.5 repetitions per second) through 31 (approximately 30 repetitions per second). The actual repeat rates are hardware-dependent and may vary from a linear scale by as much as 20%. The pvParam parameter must point to a DWORD variable that receives the setting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SystemParametersInfo の呼び出しが失敗しなければ保証されます。

成功することが保証されないので、SystemParametersInfo の戻り値チェックした方がいいです。
あと dwKeyBoardSpeed に対して 31 以下であることの assert を入れた方がいいです。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

あと SystemParametersInfo が失敗した場合の uElapse のデフォルト値も決めておいた方がいいです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

おっしゃる通りだと思います。しかし元のコードには SystemParametersInfo の呼び出しが失敗した場合の対処は入っていないので must かというとそうでもないと思います。

失敗した場合にデバッグビルドでは GetLastError 呼んで FormatMessage 呼んで OutputDebugStringW （DEBUG_TRACE 経由）呼んで、とかの対応まで求められないのであれば、こちらで追加の PR を作成して対処します。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

というか、

失敗した場合にデバッグビルドでは GetLastError 呼んで FormatMessage 呼んで OutputDebugStringW （DEBUG_TRACE 経由）呼んで

のマクロがあると良いですね…。。探した感じ既存では無さそうです。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

おっしゃる通りだと思います。しかし元のコードには SystemParametersInfo の呼び出しが失敗した場合の対処は入っていないので must かというとそうでもないと思います。

もともとのコードは nKeyBoardSpeed を初期化してないので、失敗したときの
nKeyBoardSpeed の値は不定になるので潜在バグですね。

失敗した場合にデバッグビルドでは GetLastError 呼んで FormatMessage 呼んで OutputDebugStringW （DEBUG_TRACE 経由）呼んで、とかの対応まで求められないのであれば、こちらで追加の PR を作成して対処します。

失敗したときに、dwKeyBoardSpeed あるいは uElapse の妥当なデフォルト値を
設定するようにしとけばいいと思います。

失敗した原因をわかるようにするより、失敗したときに予期しない動作を
しないようにすればいいと思います。

PR お願いします。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#527 を作成しました。

/* タイマー起動 */
if( 0 == ::SetTimer( GetHwnd(), IDT_ROLLMOUSE, nKeyBoardSpeed, EditViewTimerProc ) ){
if( 0 == ::SetTimer( GetHwnd(), IDT_ROLLMOUSE, uElapse, EditViewTimerProc ) ){
WarningMessage( GetHwnd(), LS(STR_VIEW_TIMER) );
}

Expand Down