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.

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

メニュー項目の高さ計算でマージン分をDPIに合わせて調整 #512

Merged
merged 1 commit into from
Oct 3, 2018
Merged

Conversation

beru
Copy link
Contributor

@beru beru commented Oct 1, 2018

変更前 変更後
image image

@beru
Copy link
Contributor Author

beru commented Oct 1, 2018

「メニューのドロップダウンの右向きの▶が表示されない」問題も解消されてますね。どこで描いているのか分かりませんが…。。

@beru beru mentioned this pull request Oct 1, 2018
11 tasks
@beru
Copy link
Contributor Author

beru commented Oct 1, 2018

AppVeyor が失敗するのでログを見てみると

Maximum allowed artifact storage size of 50000 Mb will be exceeded.

と最後に出ていました。

@m-tmatma
Copy link
Member

m-tmatma commented Oct 2, 2018

#514 を登録しました

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

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

LGTMです。

メニューの高さを取得できるAPIがあるはずなのでいずれ差し替えたいと思っています。いまはまず、現行仕様のままHighDPIに対応させるこの変更を入れるのがよいと考えます。

@beru
Copy link
Contributor Author

beru commented Oct 3, 2018

確認ありがとうございます。

メニューの高さを取得できるAPIがあるはずなのでいずれ差し替えたいと思っています。いまはまず、現行仕様のままHighDPIに対応させるこの変更を入れるのがよいと考えます。

これについて検索して調べてみました。

GetSystemMetrics 関数でメニュー関連で取れるのは、

  • SM_CXMENUCHECK
  • SM_CXMENUSIZE
  • SM_CYMENU
  • SM_CYMENUCHECK
  • SM_CYMENUSIZE

ですがメニューバーの高さだったり、MDIのウィンドウ閉じるボタンの高さだったりと、今回の用途には適さないようです。

他に、SystemParametersInfo 関数に SPI_GETNONCLIENTMETRICS を渡して取得する NONCLIENTMETRICS 構造体のメンバーの lfMenuFont でメニューのフォントを調べてその高さを利用する方法がありますが、それは既に現在の処理方式でした。

将来的にもっと良い方法があればそれに切り替えましょう。オーナードローでメニューを描画しているライブラリは世の中にたくさんあるのでそれらの実装を確認するのが良さそうですね。

では Merge します。もし問題が見つかったら別の PR で対処する事にしましょう。

@beru beru merged commit bb9a8fd into sakura-editor:master Oct 3, 2018
@beru beru deleted the HighDPI_MenuHeight branch October 3, 2018 00:22
@k-takata
Copy link
Member

k-takata commented Oct 3, 2018

純粋に疑問なんですが、何でオーナードローでメニューを描いているのでしょう。
最近のWindowsではデフォルトでメニューにアイコンを出せるようになっていたと思うのですが、それ以前のWindowsにも対応していたためでしょうか。それともそれ以外にも何かあります?

@ds14050
Copy link
Contributor

ds14050 commented Oct 3, 2018

CMenuDrawer.cpp の歴史の始まりです> SAKURA Editor / Code / Commit [r12] Initial revision

CMenuDrawer.cpp
Copyright (C) 1998-2000, Norio Nakatani

initial なのに revised という矛盾はさておき、答えられる人はここにはいないのではないでしょうか。

@m-tmatma
Copy link
Member

m-tmatma commented Oct 3, 2018

AppVeyor が失敗するのでログを見てみると

Maximum allowed artifact storage size of 50000 Mb will be exceeded.

と最後に出ていました。

master のビルド通りました。
#514

@m-tmatma
Copy link
Member

m-tmatma commented Oct 3, 2018

@m-tmatma m-tmatma added this to the next release milestone Oct 3, 2018
@beru
Copy link
Contributor Author

beru commented Oct 4, 2018

お、全部緑になって通ってますね。確認ありがとうございました。

@berryzplus
Copy link
Contributor

純粋に疑問なんですが、何でオーナードローでメニューを描いているのでしょう。
最近のWindowsではデフォルトでメニューにアイコンを出せるようになっていたと思うのですが、それ以前のWindowsにも対応していたためでしょうか。それともそれ以外にも何かあります?

メインの理由はそれで合ってると思います。
windows95以前のOSでメニューアイコンを表示させるためらしいです。

表示直前に表示文字列を切り替えるメニュー(ツールバーの表示/非表示など)が結構あるので、オーナードローでなければ実現できない、と考えていた可能性もあります。

windows標準の機構を使うと 区切り線 の描画方法が微妙に違ったりしますので、
OSバージョンによらずUI表示を統一したい意図があった可能性もあります。

@berryzplus
Copy link
Contributor

@k-takata さん

純粋に疑問なんですが、何でオーナードローでメニューを描いているのでしょう。

課題整理してて理由見付けました。
windows95 互換コードなので、そもそもオーナードローにしないとアイコン描けないからです。

// Jan. 29, 2002 genta
// Win95/NTが納得するsizeof( MENUITEMINFO )
// これ以外の値を与えると古いOSでちゃんと動いてくれない.
#if defined(_WIN64) || defined(_UNICODE)
static const int SIZEOF_MENUITEMINFO = sizeof(MENUITEMINFO);
#else
static const int SIZEOF_MENUITEMINFO = 44;
#endif

本来の定義 では 最後のメンバ hbmpItem を含めて48バイトになります。

メニューアイコンを描画する機能が実装されたのは windows2000 かららしいので、
windows95 で動作させる必要のあったv1系ではオーナードローするしかなかったってことかと。

@beru
Copy link
Contributor Author

beru commented Oct 8, 2018

この PR で上下のマージンの大きさを DPI に合わせる事をしましたが、メニュー項目の高さの計算は小さいアイコンサイズを GetSystemMetrics(SM_CYSMICON) で取得してそれが収まるようにしないと駄目ですね。

現状メニューやツールバー用のアイコンが 16x16ドットのものしか無くて引き延ばして表示もしていないので問題になっていませんが…。

HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 11, 2019
メニュー項目の高さ計算でマージン分をDPIに合わせて調整
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants