-
Notifications
You must be signed in to change notification settings - Fork 163
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
Windows Imaging Component を使って背景画像を読み込み、透過描画対応 #683
Conversation
背景画像のファイルパスを指定するファイルダイアログの拡張子フィルタに *.png を追加 背景画像の描画処理で透過色対応
行番号のない赤枠の部分は画像が無い方がスマートに見えるのですがいかがでしょうか?関連するissueは #665 でしょうか? |
そっすね… このPRで入れようとしてるものはかなり大きな変更です。ちょっとの変更に見せてるのはberuさんがうまいだけ。 なので、行番号背景とか描画の原点をどこにすべきかという話は分けて考えたいです。 真面目に見るつもりなんで、判断には少し時間ください。 |
わかりました。 |
誤字修正(笑 |
あれ?紛らわしい書き方ですみません。 「時間かかる」のタイムスパンは数時間のつもりです。なんとなく合意まで、1週間もかからないと予想してます。 |
MinGW ビルドコケてます。 |
master でのビルド確認のために master でビルドかけました。 |
おっけっす。問題ないと思います。 では、本題に入りましょう・・・。
この問題について、対策できる気がするので方法は探してみます。 MinGWビルドについて、必須ではないと考えていますが、今の段階で諦めるのは早すぎると思っています。WICの実装方式もやり方がいくつかあって、WRLは1手段でしかないと考えています。 とりあえず、MinGWビルドを通す方法を(勝手に)探してみます。 |
https://github.com/Microsoft/DirectXTK/wiki/ComPtr#platform-notes を読む限りでは多分大丈夫だと思います。鵜呑みにするのも良くないかもしれないし、ケースバイケースかもしれませんが。 |
調べた結果、MinGW側にも いちおう、6環境ビルド確認できてから review コメントいれるつもりです。 |
背景画像が行番号を表示する領域にまで入り込まないようにするオプションは有って良いと思います。 #659 で Issue を作った際に貼った画像では行番号の背景色による塗りつぶしが無くてそのまま背景画像が見えていますが、これは行番号の背景色とテキストの背景色を同じ色の設定にしていると特別に背景色による塗りつぶしが行われない隠れ仕様のようです(つい最近になって知りました)。 本題に戻って行番号表示領域ですが、行がまだ存在しない箇所(arigayasさんが赤枠で囲った場所)には背景色による塗りつぶしや行番号区切りが表示されていないですね。まだ行が無い場合でも背景色で塗りつぶすように設定出来るようにするのも良いかもしれないですね。 従来の動作も保てるように設定を増やして挙動を変えられるようにすると、設定項目がどんどんと増えていく事になるので、なかなか安易に機能追加をしにくい気がします。設定画面も改造しないといけないし…。 ちょっと話が変りますが、各所の色の設定で不透明度を指定出来ると良いですね。まぁしかしそうすると色々な箇所の描画に手を入れないといけなくなっちゃいそうなので実装は大変そうです。 |
おぉ、、なんか難しそう…。。近くにC++のテンプレートの名前解決に頭を悩ませている人がいたらぜひ息抜きに勧めてあげたい話題ですね。 そういえば国産の?Cのリンカってシンボルの定義が無かったり重複していてもリンカエラー出さないのが多かった気がします。が、今はもう使っていないので(゚ε゚)キニシナイ!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTMです。
細かいところで色々考えないといけないかも知れませんが、入れてしまって大丈夫だと思います。
@@ -45,6 +45,9 @@ | |||
#include <string.h> // Apr. 03, 2003 genta | |||
#include <memory> | |||
#include <OleCtl.h> | |||
#include <wincodec.h> | |||
#pragma comment(lib, "windowscodecs.lib") | |||
#include <wrl.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
レビューコメントではないですが
この3行は StdAfx.h
に移動したいな、って思ってます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sakura/sakura_core/doc/CEditDoc.cpp
Lines 45 to 46 in 070d76a
#include <string.h> // Apr. 03, 2003 genta | |
#include <memory> |
γ  ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ヽ
|彡(゚)(゚) 彡(゚)(゚) あれ?ワイらは? |
乂_______________ノ
sakura/sakura_core/doc/CEditDoc.cpp
Line 47 in 070d76a
#include <OleCtl.h> |
γ  ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ ̄ヽ
| 彡(゚)(゚) ここにいても いいの? |
乂______________ノ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
「移動」じゃなくてもいいっすよ。
まぁ #include <string.h>
は削りたいです。
@@ -317,61 +320,70 @@ void CEditDoc::SetBackgroundImage() | |||
GetInidirOrExedir( &fullPath[0], &path[0] ); | |||
path = fullPath; | |||
} | |||
const TCHAR* ext = path.GetExt(); | |||
if( 0 != auto_stricmp(ext, _T(".bmp")) ){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ: extが"bmp"と一致する場合、Bitmapを読み込んで都合よく変換する
if( iPicture ) iPicture->Release(); | ||
} | ||
}else{ | ||
m_hBackImg = (HBITMAP)::LoadImage(NULL, path.c_str(), IMAGE_BITMAP, 0, 0, LR_LOADFROMFILE | LR_CREATEDIBSECTION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ: extがbmpと一致しない場合、Bitmapとして「そのまま」読み込む
背景画像に指定したbmpファイルを削除できないのはこれが原因・・・(削られてますなw
BITMAP bmp; | ||
GetObject(m_hBackImg, sizeof(BITMAP), &bmp); | ||
m_nBackImgWidth = bmp.bmWidth; | ||
m_nBackImgHeight = bmp.bmHeight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ:bitmapを読込できた場合、サイズ情報を取得している
読込できなかった場合の考慮はなし、メッセージもなし。
} | ||
|
||
using namespace Microsoft::WRL; | ||
ComPtr<IWICImagingFactory> pIWICFactory; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ: usingに注意。ComPtrの使い方はATL/MFCのCComPtrとほぼ同じです。
Atl masterの人が「違う!」と言うかもしれませんがほぼ同じです。
if( FAILED(hr) ) return; | ||
hr = pConverter->Initialize( | ||
pFrame.Get(), | ||
GUID_WICPixelFormat32bppPBGRA, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ:WICのフォーマット定数はGUIDで、色々あります。
ここの例だと32bitプログレッシブBGRA(RGBではない)ですね。
intel cpuはlittle endianですが、little endian前提で 0xAARRGGBB
という感じに色定義できるのがBGRA形式です。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PBGRA の P はプログレッシブではなく、premultiplied alpha を示している文字かと思います。Progressive と Djent ともまた違った関係性です。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PBGRA の P はプログレッシブではなく、premultiplied alpha を示している文字かと思います。Progressive と Djent ともまた違った関係性です。
かなり長いこと勘違いしてました。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なおAPI関数の AlphaBlend
が premultiplied alpha な入力を使う仕様という事を当初把握していなくて、このPRの最初のコミットではきちんと表示されていませんでした。
自分や arigayas さんが貼り付けたスクリーンショット(自分が貼ったのは今はもう修正済み)を見て半透過部分が正しく描画されてなくて問題に気づきました。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
見映えに関する修正は通りやすいですが、
見映えが変わらない変更は了解を取り付けるのに苦労します。
WIC導入は単体で大きなインパクトを与える変更です。
が、単体で入れようとすると実質何にも変わらないので入れづらいと思ってました。
描画のとこは最悪「あとで修正」も容易なのでスルーしてました・・・
NULL, | ||
lineStride, | ||
lineStride * height, | ||
(BYTE*)pvImageBits); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ:変換済みビットマップからpixelデータを取り出してDIBSectionのビット領域にコピーしています。
ビット領域のサイズは 4bytes × 幅 × 高さになっています。
細かいことを語ると「猫でも分かる(ry」の1章分くらい説明することがたくさんありますので詳細は略。
@@ -26,6 +26,7 @@ | |||
#include "StdAfx.h" | |||
#include <vector> | |||
#include <limits.h> | |||
#pragma comment(lib, "Msimg32.lib") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ:これも StdAfx.h
に移動したいな~。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
プリコンパイル済みヘッダに変更が入るといったん全体のビルドが必要になるので抵抗感があります。
自分は何か別の cpp ファイルを1つ作って、#pragma comment(lib, "hoge.lib")
の記述はそこに纏めてしまう運用を過去に取っていました。
プロジェクトのプロパティ(Linker, Input)で指定する方法は、VS2005 とかの頃は追加すると全体のビルドが必要になってしまってうげーっと思いましたが VS2017 で試してみるとそんな事無いですね。記憶違いでしょうか…。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#pragma comment(link
の仮置き場として最も適切なのはエントリポイントがあるcppだと思います。現状でコモンコントロールの埋め込み命令がstdafx.hにあるので、とりあえずそこに合わせに行くのが妥当かな?と思ってます。
「あるべき姿は winmain.cpp
に移動」と言ってます・・・。
あるべきを語り出すと面倒くさいので放置でもいいかな、とかも思ったり。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#pragma というコンパイラ依存の記述に「あるべき場所」が存在するということに納得がいっていません。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StdAfx.h の中身は「ほぼすべてのソースファイルが必要とする基礎的なヘッダファイル」だと理解しています。画像関係は違う気がします。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#pragma
というコンパイラ依存の記述に「あるべき場所」が存在するということに納得がいっていません。
#pragma
だけならどこでもいいと思います。#pragma comment(link
なら話が別です。
実際にリンクが必要なコードと一緒に記述するか、メイン関数を含むファイルに書くべきだと思います。
StdAfx.h
の中身は「ほぼすべてのソースファイルが必要とする基礎的なヘッダファイル」だと理解しています。画像関係は違う気がします。
StdAfx.h
にはプログラムが必要とするヘッダのうち、滅多に変更されないものを記述します。プログラム内で一度でも利用され、かつ、滅多に変更されないヘッダなら、StdAfx.h
に指定する意味があります。反対に、わずかでも変更される余地のあるヘッダは StdAfx.h
から参照すべきではありません。
たくさんのソースコードで参照されるかどうかは関係ないと思います。
https://docs.microsoft.com/he-il/cpp/build/reference/creating-precompiled-header-files?view=vs-2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#pragma comment(lib, "Msimg32.lib")
等の記述場所ですが、エントリポイントがある cpp ファイルに記述するのが適切なのかどうか自分には分かりませんが、どこかのファイルにまとめて記述するのは有りだと思います。StdAfx.h
は変更すると全体のビルドが発生するので抵抗感がありますが…。
例えば、Msimg32.lib
を追加した理由は AlphaBlend
関数の為ですがこの関数を複数の cpp ファイルに今後記述するかもしれないのでそうなった場合にじゃあどっちに #pragma comment(lib, "Msimg32.lib")
を書くべきなのか、とか考えると結論が出しにくいと思うので。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stdafx.h 中に記述されているマニフェストの依存関係指定ですが、Visual Studio 2005 から使えるようになったみたいですね。
VisualStudio 2018 でMFCアプリを新規作成したところ下記の記述が Stdafx.h に入る事を確認しました。
#ifdef _UNICODE
#if defined _M_IX86
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='x86' publicKeyToken='6595b64144ccf1df' language='*'\"")
#elif defined _M_X64
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='amd64' publicKeyToken='6595b64144ccf1df' language='*'\"")
#else
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'\"")
#endif
#endif
なお以下がサクラエディタの Stdafx.h ファイル中の記述です。
#if defined _M_IX86
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='x86' publicKeyToken='6595b64144ccf1df' language='*'\"")
#elif defined _M_IA64
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='ia64' publicKeyToken='6595b64144ccf1df' language='*'\"")
#elif defined _M_X64
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='amd64' publicKeyToken='6595b64144ccf1df' language='*'\"")
#else
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'\"")
#endif
Intel の Itanium プロセッサはそもそも誰も動作確認していないので記述を打ち切って良いと思います。
https://blogs.msdn.microsoft.com/oldnewthing/20070531-00/?p=26623
に書かれてましたがそもそも processorArchitecture はワイルドカード指定で良いみたいです。
#pragma comment(linker,"/manifestdependency:\"type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'\"")
そして Stdafx.h ファイルでなくても問題無いので、今後 #pragma comment(linker, .... の記述が増える事を考えると将来的に Stdafx.h から別のファイルに移動しても良いかも知れないと思いました。ただ Stdafx.h ファイルをいじると全体のビルドが必要になるので、Stdafx.h ファイルに何か記述を追加する必要が生じた時についでに、でも良いと思います。全体のビルドもサクラエディタはそこまで重いとは思いませんが…。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stdafx.h 中に記述されているマニフェストの依存関係指定ですが、Visual Studio 2005 から使えるようになったみたいですね。
マニュフェスト自体が vista 以降だった気が。
windows xp もサービスパックを当てればマニュフェストを認識できますが。
Intel の Itanium プロセッサはそもそも誰も動作確認していないので記述を打ち切って良いと思います。
これは提案すれば反対する人いないと思います。
IA64アーキテクチャで動かすことは誰も想定してないはず。
これってどういうものですか?って質問はあるかもですが(笑
https://blogs.msdn.microsoft.com/oldnewthing/20070531-00/?p=26623
に書かれてましたがそもそも processorArchitecture はワイルドカード指定で良いみたいです。
一度どこかで話題に出た気がします。
ワイルドカードを指定した場合には、
プログラム起動時に「Win32サブシステム」が妥当なアーキテクチャに読み替えてくれます。
この読み替えのオーバーヘッドがどれくらいあるかは分かりません。
指定できるものは指定したままにしよう、と提案した記憶があります。
#pragma comment(linker
と #pragma comment(link
は別物です。
#pragma comment(linker
・・・ ソースコードにリンカオプションを埋め込む
#pragma comment(link
・・・ ソースコードに依存ライブラリ指定を埋め込む
いずれもプロジェクト設定で指定できますが、プリプロセッサで内容を変えられるのが #pragma
を使うメリットです。
リンカオプションは矛盾した指定をしてしまう可能性があるので多用すべきでない気がします。
どこか一か所にまとめて記述するようにしたら矛盾させるリスクを下げられるんじゃないかと思っています。
これは根拠がある話ではありません。なんとなく、そう考えているだけです。
@@ -198,6 +199,8 @@ void CEditView::DrawBackImage(HDC hdc, RECT& rcPaint, HDC hdcBgImg) | |||
#else | |||
CTypeSupport cTextType(this,COLORIDX_TEXT); | |||
COLORREF colorOld = ::SetBkColor(hdc, cTextType.GetBackColor()); | |||
MyFillRect(hdc, rcPaint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ: MyFillRect
は指定された矩形を現在の背景色で塗り潰す独自関数です。
透過実装を行うなら、この辺を中心に見直しかけることになると思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GDI 使った描画から Direct2D 使った描画に切替がぼちぼち必要ですね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
切替というかGdi描画機構は可能な限り残したいなぁ・・・と思ってます。
テキスト描画方法の変更に着手できるのは年明け以降になる見込みですが、直接directwrite実装に移行させたい所存です。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
直接 GDI や Direct2D, DirectWrite の API を叩かないでクラスで包んでから呼び出すようにするのが良いと思いますが、空想と現実の間には距離がありますね。要求するニーズを満たすクラスの設計も実装も大変だし面倒だし。
(BYTE*)pvImageBits); | ||
if( FAILED(hr) ){ | ||
::DeleteObject(m_hBackImg); | ||
m_hBackImg = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
メモ:ビット領域をコピーできなくても、CreateDIBSectionで作ったビットマップは削除が必要です。
Review ありがとうございました。Merge します。もし問題が見つかったら別PRで対処します。 |
Windows Imaging Component を使って背景画像を読み込み、透過描画対応
WICを使って背景画像用のファイルを読み込むように変更しました。
元実装では結構古くから存在する
OleLoadPicture
やLoadImage
といったAPI関数を使って画像ファイルを読み込んでいました。変更する利点としては対応する画像ファイルのフォーマットが増える事です。
https://docs.microsoft.com/en-us/windows/desktop/wic/-wic-about-windows-imaging-codec#native-codecs
なお、このPRの変更で背景画像のファイルパスを指定するファイルダイアログの拡張子フィルタに
*.png
を追加しました。有名な画像ファイルフォーマットで以前は読めませんでしたが今回WICを使うようにしたことで読めるようになりました。背景画像の描画処理で従来は
BitBlt
を使って描画していましたが、透過色に対応する為にAlphaBlend
を使うように変更しました。元の実装に比べると背景画像ビットマップのメモリサイズが4/3倍に増えるのと、描画する度に半透明を考慮した合成をするので処理の負荷が上がると思いますが有る程度スペックが有る現在のPCなら大きな問題にはならないかなと思います。サクラエディタの場合、背景色がベタ一色なので事前に合成した絵を作って処理の負荷を下げる事も出来ますが行いませんでした。読み込む画像によってはアルファチャンネルが存在しないので完全な無駄にはなります…。
将来的に端末エミュレータでよくあるような後ろが透けて見える表示に対応したいと思いますが、その際に背景画像を使う事でピクセル毎に透過度を変える事が出来るので見栄えの自由度が上げられます。
透過色対応の表示例
下記のアルファチャンネルを含んだ画像を背景画像に指定した場合の表示例を出します。
https://pngquant.org/firefox-512.png
テキストの背景色を白にした場合
テキストの背景色を黒にした場合
テキストの背景色を緑にした場合