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

ツールバー用のビットマップ管理を分離、結合するツールを導入する (ただし組み込みはしない) #1130

Merged

Conversation

m-tmatma
Copy link
Member

@m-tmatma m-tmatma commented Jan 4, 2020

PR の目的

#733 のために、ツールバー用のビットマップを個別に分離、結合するツールを導入する
(ただし、 この PR では 組み込みはしない)

カテゴリ

  • その他

PR の背景

もともと #680 でツールバー用のビットマップを追加しようとなったときに
管理が煩雑なので #733 でよりよい管理方法を検討しているなかで、
機能ごとにツールバーのビットマップを分離して管理して、ツールで
マージしてコンパイルしたらいいんじゃないかという案があったので
ツールを実装してみた。

PR のメリット

機能ごとにツールバーのビットマップを分離して管理するためのたたき台になる。

PR のデメリット (トレードオフとかあれば)

現状は既存のコードは変えないのでなし

PR の影響範囲

なし

関連チケット

#733
#680

参考資料

@AppVeyorBot
Copy link

Build sakura 1.0.2468 completed (commit f17d1c1465 by @m-tmatma)

@AppVeyorBot
Copy link

Build sakura 1.0.2469 completed (commit cd3f6590e2 by @m-tmatma)

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

一番右端の33列目(左端が1列目とする one-based index の数え方で)は見出し用なので分割用の Splitter と結合用の Multiplexer で下記の扱いを行ってほしいです。

  • 分割処理で一番右端の33列名は除外する
  • 結合処理で元画像の33列目を付加する

ちょっと扱いが面倒だと思いますがアイコン番号とファイル名の連番を一致させた方が良いと思います。

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

元画像と比べると結合後の画像のパレットのRGB値が微妙に変わってしまっているので、変わらないようにする必要が有ります。

分割した画像の時点で変わってしまっている事を確認しました。

例えば元画像 resource/mytool.bmp の黒色のRGB値は #010101 ですが、
image

分割後の画像だと黒色のRGB値が #000000 に変化しています。
image

@beru
Copy link
Contributor

beru commented Jan 4, 2020

分割処理では Image.Palette プロパティを設定すれば分割された連番ファイルについても同じパレットになる事を確認しました。

ただ結合処理で Graphics クラスを使う関係でフルカラーのビットマップを作成していますが Bitmap.Clome メソッドで行われる減色処理がいまいちで入力ビットマップが16色しか使っていないにも関わらず作成するビットマップの16色を少し変えてしまうようです。

考え付く対処方法ですが下記のどちらかかなと…。

  1. Graphics クラスを使わずにビットマップのピクセル操作を書いてなんとかする
  2. 結合後のビットマップは 4bit カラーにせずにフルカラーのまま出力する。どうせ組み込みはしないので、4bit カラーへの変換は別プログラムで手動で行ってもらう。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2020

2. どうせ組み込みはしないので、

これは認識違います。
この PR では組み込まないですが、将来的には組み込みを目指します。
明示的には書いてなかったですが。

2. 4bit カラーへの変換は別プログラムで手動で行ってもらう。

手作業はなくすつもりです。
そうでないと間違いが入るかもしれないのでよろしくないです。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2020

一番右端の33列目(左端が1列目とする one-based index の数え方で)は見出し用なので分割用の Splitter と結合用の Multiplexer で下記の扱いを行ってほしいです。

  • 分割処理で一番右端の33列名は除外する
  • 結合処理で元画像の33列目を付加する

ちょっと扱いが面倒だと思いますがアイコン番号とファイル名の連番を一致させた方が良いと思います。

26b123b で対応しました。

@AppVeyorBot
Copy link

Build sakura 1.0.2470 completed (commit 0943b723d4 by @m-tmatma)

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2020

58dfa2a で Palette をもと画像のものを設定するようにしてみたが、Palette の変更が反映されていないように見える。

1

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2020

分割処理では Image.Palette プロパティを設定すれば分割された連番ファイルについても同じパレットになる事を確認しました。

どういうコードで試しましたか?

@AppVeyorBot
Copy link

Build sakura 1.0.2471 completed (commit d04eeb9bfd by @m-tmatma)

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

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

右端の列の対応と分割処理のパレット対応がされたのでOKだと思います。

結合処理でパレットを変えないようにするのはまだ対応しないで良いと思います。その理由として分割された画像を後でペイントソフトで編集してから後で結合する事が考えられますが、ペイントソフトで編集する事でその画像のパレットが他の画像と変わってしまう事が考えられるからです。意図的にパレットを変えないように気を付けて編集しないとそうなります。

あと結合プログラムで連番画像を結合後にパレットの色を変えないで減色をするとなるとコード書かないといけないっぽいです(少なくとも System.Drawing.Bitmap.Clone メソッドはうまい事ケアをしてくれない)。その事もあって別PRで対応した方が良いかなと思います。

あと resource/mytool.bmp ファイルに関しては昔からのこだわりで未だに 4bpp(16色)ビットマップになっていますが、もう 8bpp(256色)やフルカラーのビットマップにしても良いと思います。フルカラーにしてしまえば減色は不要なので対応が楽ですね。

@beru
Copy link
Contributor

beru commented Jan 4, 2020

分割処理では Image.Palette プロパティを設定すれば分割された連番ファイルについても同じパレットになる事を確認しました。

どういうコードで試しましたか?

58dfa2a と同じような変更です。

#1130 (comment) で貼り付けられた画像のパレットが一致していないのは build-bmp-tools.bat の記述内容が試験用に?結合処理を経由してから再度分割をしているからだと思います。結合プログラム側での書き出し前の減色(というか PixelFormat 変換)は System.Drawing.Bitmap.Clone メソッドにお任せにしていますがそれがうまい事処理してくれない塩性能なようです。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2020

#1130 (comment) で貼り付けられた画像のパレットが一致していないのは結合処理を経由してから再度分割しているからだと思います。

その通りでした。

@m-tmatma m-tmatma added this to the v2.4.0 milestone Jan 4, 2020
@m-tmatma m-tmatma added the no-changelog 【ChangeLog除外】 label Jan 4, 2020
@m-tmatma m-tmatma merged commit 95c0515 into sakura-editor:master Jan 4, 2020
@m-tmatma m-tmatma deleted the feature/issue733-toolbar-bmp-tool branch January 4, 2020 23:51
@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 4, 2020

もう 8bpp(256色)やフルカラーのビットマップにしても良いと思います。フルカラーにしてしまえば減色は不要なので対応が楽ですね。

フルカラーにしたいですね。

@m-tmatma
Copy link
Member Author

m-tmatma commented Jan 5, 2020

あと resource/mytool.bmp ファイルに関しては昔からのこだわりで未だに 4bpp(16色)ビットマップになっていますが、もう 8bpp(256色)やフルカラーのビットマップにしても良いと思います。フルカラーにしてしまえば減色は不要なので対応が楽ですね。

my_icons.bmp をフルカラーにしたうえで、 sakura.exe と同じフォルダに置くと認識しているように見えますが、
resource\my_icons.bmp に関しても同様にフルカラー化は可能ですか?

@beru
Copy link
Contributor

beru commented Jan 5, 2020

あと resource/mytool.bmp ファイルに関しては昔からのこだわりで未だに 4bpp(16色)ビットマップになっていますが、もう 8bpp(256色)やフルカラーのビットマップにしても良いと思います。フルカラーにしてしまえば減色は不要なので対応が楽ですね。

my_icons.bmp をフルカラーにしたうえで、 sakura.exe と同じフォルダに置くと認識しているように見えますが、
resource\my_icons.bmp に関しても同様にフルカラー化は可能ですか?

動作確認しないと本当に問題がないか(フルカラーのBMPの読み込み&表示が問題ないか)は確信が持てませんが、おそらく大丈夫じゃなかったかなと思います。

なお、resource/mytool.bmp は実行ファイルに取り込まれて IDB_MYTOOL というID のビットマップリソースになります。

では resource/my_icons.bmp ファイルはどうして存在するのか?というとお飾りというか実質的には使われていません(実行ファイル内に取り込まれません)。#690 で追加されました。

GetInidirOrExedir 関数で設定ファイルや実行ファイルがあるディレクトリ内に my_icons.bmp ファイルが存在した場合にそれを読みますが、読めなかった場合は通常通りリソースの IDB_MYTOOL から読む動作になります。

@berryzplus
Copy link
Contributor

ちゃんと動作確認した記憶がないですが、 my_icons.bmp という名前の画像ファイルを、設定フォルダ(または sakura.exe と同じフォルダ)に置くと「ツールバーアイコン」として認識するように変えたような気がします。

代替画像ファイルをファイル名+拡張子で認識しているので、名前が重要っす。
my_icons.bmp の中身が実際は PNG であってもアイコンとして使えるはず。

resourcesに存在する my_icons.bmp は「入替用」を意図して突っ込んだものだった気がします。

@beru
Copy link
Contributor

beru commented Jan 5, 2020

resourcesに存在する my_icons.bmp は「入替用」を意図して突っ込んだものだった気がします。

#690 (comment) には、

これをこのままサクラエディタ本体に取り込む予定はないので、

と書かれているのでそうではなかったようです。

@berryzplus
Copy link
Contributor

berryzplus commented Jan 6, 2020

ユーザーが手で入れ替える用のファイルをリポジトリに取り込んでいます。「コピーしてリネーム」を「コピーするだけ」でできるように入れた感じです。

現状で、本体埋め込み画像を差し替える意図はありません。

@beru
Copy link
Contributor

beru commented Jan 15, 2020

ユーザーが手で入れ替える用のファイルをリポジトリに取り込んでいます。「コピーしてリネーム」を「コピーするだけ」でできるように入れた感じです。

現状で、本体埋め込み画像を差し替える意図はありません。

#690 の説明には

取り込みを行うメリットとして、今後の変更を記録できるようになります。

と書かれているので、ユーザーが手で入れ替え、っていうのを目的としていたわけでもないような…。

ユーザー というのも結構概念が広い言葉ですが(開発者も含まれる)、一般のユーザーがgitリポジトリをcloneする事は考えづらいです。現時点ではインストーラーに my_icons.bmp は含めていないのでそこまで気軽に入れ替えが行えるわけでは無い気もします。

まぁ 多分~だったんじゃないかな というコメントに対してツッコミを繰り返しても最終的には うっざ に至ってしまうだろうという議論の不毛感があります。

なお個人的に感じている resource/my_icons.bmp が追加された事のメリットとしては、8ビットインデックスカラー画像のテストがやりやすい事です。今アプリで使われている resource/mytool.bmp は4ビットインデックスカラー画像なので、他のフォーマットの画像の動作確認をする際に役立ちます。

HoppingTappy pushed a commit to HoppingTappy/sakura that referenced this pull request Jun 16, 2020
…toolbar-bmp-tool

ツールバー用のビットマップ管理を分離、結合するツールを導入する (ただし組み込みはしない)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants